Messages in this thread | | | Date | Tue, 28 May 2019 08:06:40 -0700 | From | Guenter Roeck <> | Subject | Re: [PATCH 2/2] hwmon: core: fix potential memory leak in *hwmon_device_register* |
| |
Hi Eduardo,
On Fri, May 17, 2019 at 04:13:37PM -0700, Eduardo Valentin wrote: > When registering a hwmon device with HWMON_C_REGISTER_TZ flag > in place, the hwmon subsystem will attempt to register the device > also with the thermal subsystem. When the of-thermal registration > fails, __hwmon_device_register jumps to ida_remove, leaving > the locally allocated hwdev pointer and also the hdev registered. > > This patch fixes both issues by jumping to a new label that > will first unregister hdev and the fall into the kfree of hwdev > to finally remove the idas and propagate the error code. > > Cc: Jean Delvare <jdelvare@suse.com> > Cc: Guenter Roeck <linux@roeck-us.net> > Cc: linux-hwmon@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Eduardo Valentin <eduval@amazon.com> > --- > drivers/hwmon/hwmon.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c > index 6b3559f58b67..6f1194952189 100644 > --- a/drivers/hwmon/hwmon.c > +++ b/drivers/hwmon/hwmon.c > @@ -637,7 +637,7 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata, > hwdev, j); > if (err) { > device_unregister(hdev); > - goto ida_remove; > + goto device_unregister;
Good find, but device_unregister() is already called above. You need to either remove that, or replace the goto to point to free_hwmon. The new label would probably the cleaner solution since it follows the coding style.
Thanks Guenter
> } > } > } > @@ -646,6 +646,8 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata, > > return hdev; > > +device_unregister: > + device_unregister(hdev); > free_hwmon: > kfree(hwdev); > ida_remove: > -- > 2.21.0 >
| |