On 10/08/17 15:02, Hans Verkuil wrote:
On 09/08/17 13:15, Sakari Ailus wrote:
From: Rui Miguel Silva rmfrfs@gmail.com
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.
I'm not sure why this is part of this patch series. This is a greybus bug fix, right? And independent from the other two patches.
Ah, I see. The second patch sits on top of this change. Sorry, I was a bit too quick there.
Hans
Fixes: 2870b52bae4c ("greybus: lights: add lights implementation") Reported-by: Sakari Ailus sakari.ailus@linux.intel.com Signed-off-by: Rui Miguel Silva rmfrfs@gmail.com Reviewed-by: Viresh Kumar viresh.kumar@linaro.org
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..0771db418f84 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 = &connection->bundle->dev;
- struct v4l2_flash_config *sd_cfg;
- struct v4l2_flash_config sd_cfg = { {0} };
Just use '= {};'
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(&channel_torch->intensity_uA,
&sd_cfg->torch_intensity);
&sd_cfg.torch_intensity);channel_ind = get_channel_from_mode(light, GB_CHANNEL_MODE_INDICATOR); if (channel_ind) { __gb_lights_channel_v4l2_config(&channel_ind->intensity_uA,
&sd_cfg->indicator_intensity);
&sd_cfg.indicator_intensity);@@ -561,27 +556,21 @@ static int gb_lights_light_v4l2_register(struct gb_light *light) fled = &channel_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,
&v4l2_flash_ops, sd_cfg);- if (IS_ERR_OR_NULL(light->v4l2_flash)) {
ret = PTR_ERR(light->v4l2_flash);goto out_free;- }
&v4l2_flash_ops, &sd_cfg);- if (IS_ERR_OR_NULL(light->v4l2_flash))
Just IS_ERR since v4l2_flash_init() never returns NULL.
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)
After those two changes:
Acked-by: Hans Verkuil hans.verkuil@cisco.com
Regards,
Hans