| Message ID | 20200502211056.20975-1-wu000273@umn.edu (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | power: supply: fix memory leaks | expand |
Hi, On Sat, May 02, 2020 at 04:10:56PM -0500, wu000273@umn.edu wrote: > From: Qiushi Wu <wu000273@umn.edu> > > In function power_supply_add_hwmon_sysfs(), psyhw->props is > allocated by bitmap_zalloc(). But this pointer is not deallocated > in several error paths, which lead to memory leak bugs. To fix > this, we can call bitmap_free() to free this pointer. > > Signed-off-by: Qiushi Wu <wu000273@umn.edu> > --- You are correct, that there is a problem in the first instance, but the other changes are incorrect and introduce a new double free. Please read documentation for devm_add_action(). The proper fix is to just replace the call to devm_add_action() with devm_add_action_or_reset(). Please send a new version for this. Also please add the following tag: Fixes: e67d4dfc9ff19 ("power: supply: Add HWMON compatibility layer") Thanks, -- Sebastian > drivers/power/supply/power_supply_hwmon.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/power/supply/power_supply_hwmon.c b/drivers/power/supply/power_supply_hwmon.c > index 75cf861ba492..7453390ab7a4 100644 > --- a/drivers/power/supply/power_supply_hwmon.c > +++ b/drivers/power/supply/power_supply_hwmon.c > @@ -307,7 +307,7 @@ int power_supply_add_hwmon_sysfs(struct power_supply *psy) > ret = devm_add_action(dev, power_supply_hwmon_bitmap_free, > psyhw->props); > if (ret) > - goto error; > + goto out_free; > > for (i = 0; i < desc->num_properties; i++) { > const enum power_supply_property prop = desc->properties[i]; > @@ -342,7 +342,7 @@ int power_supply_add_hwmon_sysfs(struct power_supply *psy) > new_name = devm_kstrdup(dev, name, GFP_KERNEL); > if (!new_name) { > ret = -ENOMEM; > - goto error; > + goto out_free; > } > strreplace(new_name, '-', '_'); > name = new_name; > @@ -353,10 +353,12 @@ int power_supply_add_hwmon_sysfs(struct power_supply *psy) > NULL); > ret = PTR_ERR_OR_ZERO(hwmon); > if (ret) > - goto error; > + goto out_free; > > devres_close_group(dev, power_supply_add_hwmon_sysfs); > return 0; > +out_free: > + bitmap_free(psyhw->props); > error: > devres_release_group(dev, NULL); > return ret; > -- > 2.17.1 >
diff --git a/drivers/power/supply/power_supply_hwmon.c b/drivers/power/supply/power_supply_hwmon.c index 75cf861ba492..7453390ab7a4 100644 --- a/drivers/power/supply/power_supply_hwmon.c +++ b/drivers/power/supply/power_supply_hwmon.c @@ -307,7 +307,7 @@ int power_supply_add_hwmon_sysfs(struct power_supply *psy) ret = devm_add_action(dev, power_supply_hwmon_bitmap_free, psyhw->props); if (ret) - goto error; + goto out_free; for (i = 0; i < desc->num_properties; i++) { const enum power_supply_property prop = desc->properties[i]; @@ -342,7 +342,7 @@ int power_supply_add_hwmon_sysfs(struct power_supply *psy) new_name = devm_kstrdup(dev, name, GFP_KERNEL); if (!new_name) { ret = -ENOMEM; - goto error; + goto out_free; } strreplace(new_name, '-', '_'); name = new_name; @@ -353,10 +353,12 @@ int power_supply_add_hwmon_sysfs(struct power_supply *psy) NULL); ret = PTR_ERR_OR_ZERO(hwmon); if (ret) - goto error; + goto out_free; devres_close_group(dev, power_supply_add_hwmon_sysfs); return 0; +out_free: + bitmap_free(psyhw->props); error: devres_release_group(dev, NULL); return ret;