Re: [PATCH v3 1/3] staging: greybus: light: fix memory leak in v4l2 register

[Greg Kroah-Hartman]

On Thu, Aug 10, 2017 at 06:49:45PM +0300, Sakari Ailus wrote:
> From: Rui Miguel Silva 
> 
> We are allocating memory for the v4l2 flash configuration structure and
> leak it in the normal path. Just use the stack for this as we do not
> use it outside of this function.
> 
> Also use IS_ERR() instead of IS_ERR_OR_NULL() to check return value from
> v4l2_flash_init() for it never returns NULL.
> 
> Fixes: 2870b52bae4c ("greybus: lights: add lights implementation")
> Reported-by: Sakari Ailus 
> Signed-off-by: Rui Miguel Silva 
> Reviewed-by: Viresh Kumar 
> Signed-off-by: Sakari Ailus 
> Acked-by: Hans Verkuil 
> ---
>  drivers/staging/greybus/light.c | 29 +
>  1 file changed, 9 insertions(+), 20 deletions(-)

Does not apply to my tree :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 1/3] staging: greybus: light: fix memory leak in v4l2 register

[Sakari Ailus]

From: Rui Miguel Silva 

We are allocating memory for the v4l2 flash configuration structure and
leak it in the normal path. Just use the stack for this as we do not
use it outside of this function.

Also use IS_ERR() instead of IS_ERR_OR_NULL() to check return value from
v4l2_flash_init() for it never returns NULL.

Fixes: 2870b52bae4c ("greybus: lights: add lights implementation")
Reported-by: Sakari Ailus 
Signed-off-by: Rui Miguel Silva 
Reviewed-by: Viresh Kumar 
Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
---
 drivers/staging/greybus/light.c | 29 +
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
index 129ceed39829..81469d087e74 100644
--- a/drivers/staging/greybus/light.c
+++ b/drivers/staging/greybus/light.c
@@ -534,25 +534,20 @@ static int gb_lights_light_v4l2_register(struct gb_light 
*light)
 {
struct gb_connection *connection = get_conn_from_light(light);
struct device *dev = >bundle->dev;
-   struct v4l2_flash_config *sd_cfg;
+   struct v4l2_flash_config sd_cfg = { {0} };
struct led_classdev_flash *fled;
struct led_classdev *iled = NULL;
struct gb_channel *channel_torch, *channel_ind, *channel_flash;
-   int ret = 0;
-
-   sd_cfg = kcalloc(1, sizeof(*sd_cfg), GFP_KERNEL);
-   if (!sd_cfg)
-   return -ENOMEM;
 
channel_torch = get_channel_from_mode(light, GB_CHANNEL_MODE_TORCH);
if (channel_torch)
__gb_lights_channel_v4l2_config(_torch->intensity_uA,
-   _cfg->torch_intensity);
+   _cfg.torch_intensity);
 
channel_ind = get_channel_from_mode(light, GB_CHANNEL_MODE_INDICATOR);
if (channel_ind) {
__gb_lights_channel_v4l2_config(_ind->intensity_uA,
-   _cfg->indicator_intensity);
+   _cfg.indicator_intensity);
iled = _ind->fled.led_cdev;
}
 
@@ -561,27 +556,21 @@ static int gb_lights_light_v4l2_register(struct gb_light 
*light)
 
fled = _flash->fled;
 
-   snprintf(sd_cfg->dev_name, sizeof(sd_cfg->dev_name), "%s", light->name);
+   snprintf(sd_cfg.dev_name, sizeof(sd_cfg.dev_name), "%s", light->name);
 
/* Set the possible values to faults, in our case all faults */
-   sd_cfg->flash_faults = LED_FAULT_OVER_VOLTAGE | LED_FAULT_TIMEOUT |
+   sd_cfg.flash_faults = LED_FAULT_OVER_VOLTAGE | LED_FAULT_TIMEOUT |
LED_FAULT_OVER_TEMPERATURE | LED_FAULT_SHORT_CIRCUIT |
LED_FAULT_OVER_CURRENT | LED_FAULT_INDICATOR |
LED_FAULT_UNDER_VOLTAGE | LED_FAULT_INPUT_VOLTAGE |
LED_FAULT_LED_OVER_TEMPERATURE;
 
light->v4l2_flash = v4l2_flash_init(dev, NULL, fled, iled,
-   _flash_ops, sd_cfg);
-   if (IS_ERR_OR_NULL(light->v4l2_flash)) {
-   ret = PTR_ERR(light->v4l2_flash);
-   goto out_free;
-   }
+   _flash_ops, _cfg);
+   if (IS_ERR(light->v4l2_flash))
+   return PTR_ERR(light->v4l2_flash);
 
-   return ret;
-
-out_free:
-   kfree(sd_cfg);
-   return ret;
+   return 0;
 }
 
 static void gb_lights_light_v4l2_unregister(struct gb_light *light)
-- 
2.11.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel