On Tue, May 16, 2017 at 02:28:28PM -0700, Andrey Smirnov wrote: > On Tue, May 16, 2017 at 6:44 AM, Johan Hovold <[email protected]> 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 <[email protected]> # 4.3 > > Cc: Andrew Lunn <[email protected]> > > Cc: Srinivas Kandagatla <[email protected]> > > Cc: Mika Westerberg <[email protected]> > > Signed-off-by: Johan Hovold <[email protected]> > > --- > > I submitted identical patch to fix this about a month ago here: > > lkml.kernel.org/r/[email protected]
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 <[email protected]> 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 > >
