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

[Johan Hovold <johan@kernel.org>]

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@vger.kernel.org> know about it.
> 
> 
> From 20777bc57c346b6994f465e0d8261a7fbf213a09 Mon Sep 17 00:00:00 2001
> From: Johan Hovold <johan@kernel.org>
> Date: Thu, 30 Mar 2017 12:15:35 +0200
> Subject: NFC: fix broken device allocation
> 
> From: Johan Hovold <johan@kernel.org>
> 
> 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@linux.intel.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> ---
>  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@kernel.org

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.

Johan