Re: Patch "NFC: fix broken device allocation" has been added to the 4.12-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Jul 22, 2017 at 03:25:12PM +0200, Johan Hovold wrote:
> On Sat, Jul 22, 2017 at 03:02:12PM +0200, Greg Kroah-Hartman wrote:
> > 
> > This is a note to let you know that I've just added the patch titled
> > 
> >     NFC: fix broken device allocation
> > 
> > to the 4.12-stable tree which can be found at:
> >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > 
> > The filename of the patch is:
> >      nfc-fix-broken-device-allocation.patch
> > and it can be found in the queue-4.12 subdirectory.
> > 
> > If you, or anyone else, feels it should not be added to the stable tree,
> > please let <stable@xxxxxxxxxxxxxxx> know about it.
> > 
> > 
> > From 20777bc57c346b6994f465e0d8261a7fbf213a09 Mon Sep 17 00:00:00 2001
> > From: Johan Hovold <johan@xxxxxxxxxx>
> > Date: Thu, 30 Mar 2017 12:15:35 +0200
> > Subject: NFC: fix broken device allocation
> > 
> > From: Johan Hovold <johan@xxxxxxxxxx>
> > 
> > commit 20777bc57c346b6994f465e0d8261a7fbf213a09 upstream.
> > 
> > Commit 7eda8b8e9677 ("NFC: Use IDR library to assing NFC devices IDs")
> > moved device-id allocation and struct-device initialisation from
> > nfc_allocate_device() to nfc_register_device().
> > 
> > This broke just about every nfc-device-registration error path, which
> > continue to call nfc_free_device() that tries to put the device
> > reference of the now uninitialised (but zeroed) struct device:
> > 
> > kobject: '(null)' (ce316420): is not initialized, yet kobject_put() is being called.
> > 
> > The late struct-device initialisation also meant that various work
> > queues whose names are derived from the nfc device name were also
> > misnamed:
> > 
> >   421 root         0 SW<  [(null)_nci_cmd_]
> >   422 root         0 SW<  [(null)_nci_rx_w]
> >   423 root         0 SW<  [(null)_nci_tx_w]
> > 
> > Move the id-allocation and struct-device initialisation back to
> > nfc_allocate_device() and fix up the single call site which did not use
> > nfc_free_device() in its error path.
> > 
> > Fixes: 7eda8b8e9677 ("NFC: Use IDR library to assing NFC devices IDs")
> > Cc: Samuel Ortiz <sameo@xxxxxxxxxxxxxxx>
> > Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
> > Signed-off-by: Samuel Ortiz <sameo@xxxxxxxxxxxxxxx>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > 
> > ---
> >  net/nfc/core.c     |   31 ++++++++++++++++++-------------
> >  net/nfc/nci/core.c |    3 +--
> >  2 files changed, 19 insertions(+), 15 deletions(-)
> > 
> > --- a/net/nfc/core.c
> > +++ b/net/nfc/core.c
> > @@ -982,6 +982,8 @@ static void nfc_release(struct device *d
> >  			kfree(se);
> >  	}
> >  
> > +	ida_simple_remove(&nfc_index_ida, dev->idx);
> > +
> >  	kfree(dev);
> >  }
> >  
> > @@ -1056,6 +1058,7 @@ struct nfc_dev *nfc_allocate_device(stru
> >  				    int tx_headroom, int tx_tailroom)
> >  {
> >  	struct nfc_dev *dev;
> > +	int rc;
> >  
> >  	if (!ops->start_poll || !ops->stop_poll || !ops->activate_target ||
> >  	    !ops->deactivate_target || !ops->im_transceive)
> > @@ -1068,6 +1071,15 @@ struct nfc_dev *nfc_allocate_device(stru
> >  	if (!dev)
> >  		return NULL;
> >  
> > +	rc = ida_simple_get(&nfc_index_ida, 0, 0, GFP_KERNEL);
> > +	if (rc < 0)
> > +		goto err_free_dev;
> > +	dev->idx = rc;
> > +
> > +	dev->dev.class = &nfc_class;
> > +	dev_set_name(&dev->dev, "nfc%d", dev->idx);
> > +	device_initialize(&dev->dev);
> > +
> >  	dev->ops = ops;
> >  	dev->supported_protocols = supported_protocols;
> >  	dev->tx_headroom = tx_headroom;
> > @@ -1090,6 +1102,11 @@ struct nfc_dev *nfc_allocate_device(stru
> >  	}
> >  
> >  	return dev;
> > +
> > +err_free_dev:
> > +	kfree(dev);
> > +
> > +	return ERR_PTR(rc);
> 
> This should have been "return NULL"; a follow up fix has been posted
> to address this, but it has not yet been applied (Samuel?):
> 
> 	lkml.kernel.org/r/20170709110858.20331-1-johan@xxxxxxxxxx
> 
> Since we would only hit this error path after an ida allocation failure
> (out of memory), I'd say it's fine to address this in -stable in a
> follow-up backport once that fix is upstream however.

Yeah, a follow-up patch to match what is in Linus's tree sounds like a
good idea.

thanks,

greg k-h



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]