From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753927AbdEQIvM (ORCPT ); Wed, 17 May 2017 04:51:12 -0400 Received: from mail-wr0-f193.google.com ([209.85.128.193]:34546 "EHLO mail-wr0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753504AbdEQIvH (ORCPT ); Wed, 17 May 2017 04:51:07 -0400 Date: Wed, 17 May 2017 10:51:05 +0200 From: Johan Hovold To: Andrey Smirnov Cc: Johan Hovold , Srinivas Kandagatla , linux-kernel , stable , Andrew Lunn , Mika Westerberg Subject: Re: [PATCH] nvmem: core: fix leaks on registration errors Message-ID: <20170517085105.GR3657@localhost> References: <20170516134459.16698-1-johan@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 16, 2017 at 02:28:28PM -0700, Andrey Smirnov wrote: > On Tue, May 16, 2017 at 6:44 AM, Johan Hovold 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 # 4.3 > > Cc: Andrew Lunn > > Cc: Srinivas Kandagatla > > Cc: Mika Westerberg > > Signed-off-by: Johan Hovold > > --- > > 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 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 > >