Messages in this thread Patch in this message | | | Date | Wed, 17 May 2017 10:51:05 +0200 | From | Johan Hovold <> | Subject | Re: [PATCH] nvmem: core: fix leaks on registration errors |
| |
On Tue, May 16, 2017 at 02:28:28PM -0700, Andrey Smirnov wrote: > On Tue, May 16, 2017 at 6:44 AM, Johan Hovold <johan@kernel.org> wrote: > > Make sure to deregister and release the nvmem device and underlying > > memory on registration errors. > > > > Note that the private data must be freed using put_device() once the > > struct device has been initialised. > > > > Also note that there's a related reference leak in the deregistration > > function as reported by Mika Westerberg which is being fixed separately. > > > > Fixes: b6c217ab9be6 ("nvmem: Add backwards compatibility support for older EEPROM drivers.") > > Fixes: eace75cfdcf7 ("nvmem: Add a simple NVMEM framework for nvmem providers") > > Cc: stable <stable@vger.kernel.org> # 4.3 > > Cc: Andrew Lunn <andrew@lunn.ch> > > Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > > Signed-off-by: Johan Hovold <johan@kernel.org> > > --- > > I submitted identical patch to fix this about a month ago here: > > lkml.kernel.org/r/20170418142454.23921-1-andrew.smirnov@gmail.com
Oh, and you fixed up both the registration and deregistration leaks in your series too. Looks like the maintainer is on vacation now so perhaps we'll see a third independent fix for these bugs soon. ;)
As for your series, I prefer this version for nvmem_register due to the difference in error label naming (named after what they do rather than where they are used), but as I also mentioned to Mika, I think just adding the missing put_device to nvmem_unregister as you did is preferred.
But the important thing is that this gets fixed.
> so, FWIW, > > Acked-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Thanks, Johan
> > drivers/nvmem/core.c | 13 ++++++++----- > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c > > index 8c830a80a648..6cf916d9db6d 100644 > > --- a/drivers/nvmem/core.c > > +++ b/drivers/nvmem/core.c > > @@ -489,21 +489,24 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config) > > > > rval = device_add(&nvmem->dev); > > if (rval) > > - goto out; > > + goto err_put_device; > > > > if (config->compat) { > > rval = nvmem_setup_compat(nvmem, config); > > if (rval) > > - goto out; > > + goto err_device_del; > > } > > > > if (config->cells) > > nvmem_add_cells(nvmem, config); > > > > return nvmem; > > -out: > > - ida_simple_remove(&nvmem_ida, nvmem->id); > > - kfree(nvmem); > > + > > +err_device_del: > > + device_del(&nvmem->dev); > > +err_put_device: > > + put_device(&nvmem->dev); > > + > > return ERR_PTR(rval); > > } > > EXPORT_SYMBOL_GPL(nvmem_register); > > -- > > 2.13.0 > >
| |