greybus-dev January 2020

greybus-dev@lists.linaro.org

6 participants
5 discussions

[PATCH] staging: greybus: bootrom: fix uninitialized variables

by Saurav Girepunje

fix uninitialized variables issue found using static code analysis tool (error) Uninitialized variable: offset (error) Uninitialized variable: size Signed-off-by: Saurav Girepunje <saurav.girepunje(a)gmail.com> --- drivers/staging/greybus/bootrom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c index a8efb86..9eabeb3 100644 --- a/drivers/staging/greybus/bootrom.c +++ b/drivers/staging/greybus/bootrom.c @@ -245,7 +245,7 @@ static int gb_bootrom_get_firmware(struct gb_operation *op) struct gb_bootrom_get_firmware_request *firmware_request; struct gb_bootrom_get_firmware_response *firmware_response; struct device *dev = &op->connection->bundle->dev; - unsigned int offset, size; + unsigned int offset = 0, size = 0; enum next_request_type next_request; int ret = 0; -- 1.9.1

5 years

[PATCH] staging: greybus: fix fw is NULL but dereferenced.

by Saurav Girepunje

Fix the warning reported by cocci check. Changes: In queue_work fw dereference before it actually get assigned. move queue_work before gb_bootrom_set_timeout. As gb_bootrom_get_firmware () return NEXT_REQ_READY_TO_BOOT only when there is no error and offset + size is actually equal to fw->size. So initialized next_request to NEXT_REQ_GET_FIRMWARE for return in other case. Signed-off-by: Saurav Girepunje <saurav.girepunje(a)gmail.com> --- drivers/staging/greybus/bootrom.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c index a8efb86..f54514e 100644 --- a/drivers/staging/greybus/bootrom.c +++ b/drivers/staging/greybus/bootrom.c @@ -246,7 +246,7 @@ static int gb_bootrom_get_firmware(struct gb_operation *op) struct gb_bootrom_get_firmware_response *firmware_response; struct device *dev = &op->connection->bundle->dev; unsigned int offset, size; - enum next_request_type next_request; + enum next_request_type next_request = NEXT_REQ_GET_FIRMWARE; int ret = 0; /* Disable timeouts */ @@ -296,13 +296,11 @@ static int gb_bootrom_get_firmware(struct gb_operation *op) unlock: mutex_unlock(&bootrom->mutex); -queue_work: /* Refresh timeout */ if (!ret && (offset + size == fw->size)) next_request = NEXT_REQ_READY_TO_BOOT; - else - next_request = NEXT_REQ_GET_FIRMWARE; +queue_work: gb_bootrom_set_timeout(bootrom, next_request, NEXT_REQ_TIMEOUT_MS); return ret; -- 1.9.1

5 years

[PATCH AUTOSEL 4.9 193/251] staging: greybus: light: fix a couple double frees

by Sasha Levin

From: Dan Carpenter <dan.carpenter(a)oracle.com> [ Upstream commit 329101244f214952606359d254ae883b7109e1a5 ] The problem is in gb_lights_request_handler(). If we get a request to change the config then we release the light with gb_lights_light_release() and re-allocated it. However, if the allocation fails part way through then we call gb_lights_light_release() again. This can lead to a couple different double frees where we haven't cleared out the original values: gb_lights_light_v4l2_unregister(light); ... kfree(light->channels); kfree(light->name); I also made a small change to how we set "light->channels_count = 0;". The original code handled this part fine and did not cause a use after free but it was sort of complicated to read. Fixes: 2870b52bae4c ("greybus: lights: add lights implementation") Signed-off-by: Dan Carpenter <dan.carpenter(a)oracle.com> Acked-by: Rui Miguel Silva <rmfrfs(a)gmail.com> Link: https://lore.kernel.org/r/20190829122839.GA20116@mwanda Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/staging/greybus/light.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c index 9f01427f35f9..1cb82cc28aa7 100644 --- a/drivers/staging/greybus/light.c +++ b/drivers/staging/greybus/light.c @@ -1102,21 +1102,21 @@ static void gb_lights_channel_release(struct gb_channel *channel) static void gb_lights_light_release(struct gb_light *light) { int i; - int count; light->ready = false; - count = light->channels_count; - if (light->has_flash) gb_lights_light_v4l2_unregister(light); + light->has_flash = false; - for (i = 0; i < count; i++) { + for (i = 0; i < light->channels_count; i++) gb_lights_channel_release(&light->channels[i]); - light->channels_count--; - } + light->channels_count = 0; + kfree(light->channels); + light->channels = NULL; kfree(light->name); + light->name = NULL; } static void gb_lights_release(struct gb_lights *glights) -- 2.20.1

5 years

[PATCH AUTOSEL 4.14 283/371] staging: greybus: light: fix a couple double frees

by Sasha Levin

From: Dan Carpenter <dan.carpenter(a)oracle.com> [ Upstream commit 329101244f214952606359d254ae883b7109e1a5 ] The problem is in gb_lights_request_handler(). If we get a request to change the config then we release the light with gb_lights_light_release() and re-allocated it. However, if the allocation fails part way through then we call gb_lights_light_release() again. This can lead to a couple different double frees where we haven't cleared out the original values: gb_lights_light_v4l2_unregister(light); ... kfree(light->channels); kfree(light->name); I also made a small change to how we set "light->channels_count = 0;". The original code handled this part fine and did not cause a use after free but it was sort of complicated to read. Fixes: 2870b52bae4c ("greybus: lights: add lights implementation") Signed-off-by: Dan Carpenter <dan.carpenter(a)oracle.com> Acked-by: Rui Miguel Silva <rmfrfs(a)gmail.com> Link: https://lore.kernel.org/r/20190829122839.GA20116@mwanda Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/staging/greybus/light.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c index 0f538b8c3a07..4e7575147775 100644 --- a/drivers/staging/greybus/light.c +++ b/drivers/staging/greybus/light.c @@ -1103,21 +1103,21 @@ static void gb_lights_channel_release(struct gb_channel *channel) static void gb_lights_light_release(struct gb_light *light) { int i; - int count; light->ready = false; - count = light->channels_count; - if (light->has_flash) gb_lights_light_v4l2_unregister(light); + light->has_flash = false; - for (i = 0; i < count; i++) { + for (i = 0; i < light->channels_count; i++) gb_lights_channel_release(&light->channels[i]); - light->channels_count--; - } + light->channels_count = 0; + kfree(light->channels); + light->channels = NULL; kfree(light->name); + light->name = NULL; } static void gb_lights_release(struct gb_lights *glights) -- 2.20.1

5 years

[PATCH AUTOSEL 4.19 518/671] staging: greybus: light: fix a couple double frees

by Sasha Levin

From: Dan Carpenter <dan.carpenter(a)oracle.com> [ Upstream commit 329101244f214952606359d254ae883b7109e1a5 ] The problem is in gb_lights_request_handler(). If we get a request to change the config then we release the light with gb_lights_light_release() and re-allocated it. However, if the allocation fails part way through then we call gb_lights_light_release() again. This can lead to a couple different double frees where we haven't cleared out the original values: gb_lights_light_v4l2_unregister(light); ... kfree(light->channels); kfree(light->name); I also made a small change to how we set "light->channels_count = 0;". The original code handled this part fine and did not cause a use after free but it was sort of complicated to read. Fixes: 2870b52bae4c ("greybus: lights: add lights implementation") Signed-off-by: Dan Carpenter <dan.carpenter(a)oracle.com> Acked-by: Rui Miguel Silva <rmfrfs(a)gmail.com> Link: https://lore.kernel.org/r/20190829122839.GA20116@mwanda Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/staging/greybus/light.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c index 010ae1e9c7fb..40680eaf3974 100644 --- a/drivers/staging/greybus/light.c +++ b/drivers/staging/greybus/light.c @@ -1098,21 +1098,21 @@ static void gb_lights_channel_release(struct gb_channel *channel) static void gb_lights_light_release(struct gb_light *light) { int i; - int count; light->ready = false; - count = light->channels_count; - if (light->has_flash) gb_lights_light_v4l2_unregister(light); + light->has_flash = false; - for (i = 0; i < count; i++) { + for (i = 0; i < light->channels_count; i++) gb_lights_channel_release(&light->channels[i]); - light->channels_count--; - } + light->channels_count = 0; + kfree(light->channels); + light->channels = NULL; kfree(light->name); + light->name = NULL; } static void gb_lights_release(struct gb_lights *glights) -- 2.20.1

5 years

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

greybus-dev January 2020