Skip to contentSkip to conversation

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

PATCH [email protected] 2 participants 3 messages Jul 22, 2017

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

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 <[email protected]> know about it.

Quoted 1 line

From 20777bc57c346b6994f465e0d8261a7fbf213a09 Mon Sep 17 00:00:00 2001

From: Johan Hovold <[email protected]>
Date: Thu, 30 Mar 2017 12:15:35 +0200
Subject: NFC: fix broken device allocation

From: Johan Hovold <[email protected]>

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 <[email protected]>
---
 net/nfc/core.c     |   31 ++++++++++++++++++-------------
 net/nfc/nci/core.c |    3 +--
 2 files changed, 19 insertions(+), 15 deletions(-)
net/nfc/core.c +18 -13
@@ -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);
 }
 EXPORT_SYMBOL(nfc_allocate_device);
 
@@ -1104,14 +1121,6 @@ int nfc_register_device(struct nfc_dev *
 
 	pr_debug("dev_name=%s\n", dev_name(&dev->dev));
 
-	dev->idx = ida_simple_get(&nfc_index_ida, 0, 0, GFP_KERNEL);
-	if (dev->idx < 0)
-		return dev->idx;
-
-	dev->dev.class = &nfc_class;
-	dev_set_name(&dev->dev, "nfc%d", dev->idx);
-	device_initialize(&dev->dev);
-
 	mutex_lock(&nfc_devlist_mutex);
 	nfc_devlist_generation++;
 	rc = device_add(&dev->dev);
@@ -1149,12 +1158,10 @@ EXPORT_SYMBOL(nfc_register_device);
  */
 void nfc_unregister_device(struct nfc_dev *dev)
 {
-	int rc, id;
+	int rc;
 
 	pr_debug("dev_name=%s\n", dev_name(&dev->dev));
 
-	id = dev->idx;
-
 	if (dev->rfkill) {
 		rfkill_unregister(dev->rfkill);
 		rfkill_destroy(dev->rfkill);
@@ -1179,8 +1186,6 @@ void nfc_unregister_device(struct nfc_de
 	nfc_devlist_generation++;
 	device_del(&dev->dev);
 	mutex_unlock(&nfc_devlist_mutex);
-
-	ida_simple_remove(&nfc_index_ida, id);
 }
 EXPORT_SYMBOL(nfc_unregister_device);
 
net/nfc/nci/core.c +1 -2
@@ -1173,8 +1173,7 @@ struct nci_dev *nci_allocate_device(stru
 	return ndev;
 
 free_nfc:
-	kfree(ndev->nfc_dev);
-
+	nfc_free_device(ndev->nfc_dev);
 free_nci:
 	kfree(ndev);
 	return NULL;

Patches currently in stable-queue which might be from [email protected] are

queue-4.12/nfc-nfcmrvl-use-nfc-device-for-firmware-download.patch queue-4.12/nfc-nfcmrvl-fix-firmware-management-initialisation.patch queue-4.12/nfc-nfcmrvl-do-not-use-device-managed-resources.patch queue-4.12/thermal-max77620-fix-device-node-reference-imbalance.patch queue-4.12/nfc-nfcmrvl_uart-add-missing-tty-device-sanity-check.patch queue-4.12/nfc-fix-broken-device-allocation.patch

#1 lore raw
Quoted 94 lines · Greg Kroah-Hartman

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 <[email protected]> know about it.

From 20777bc57c346b6994f465e0d8261a7fbf213a09 Mon Sep 17 00:00:00 2001
From: Johan Hovold <[email protected]>
Date: Thu, 30 Mar 2017 12:15:35 +0200
Subject: NFC: fix broken device allocation

From: Johan Hovold <[email protected]>

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 <[email protected]>
---
 net/nfc/core.c     |   31 ++++++++++++++++++-------------
 net/nfc/nci/core.c |    3 +--
 2 files changed, 19 insertions(+), 15 deletions(-)
net/nfc/core.c +12 -0
@@ -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;

net/nfc/core.c struct nfc_dev *nfc_allocate_device(stru · line 1109

Quoted 8 lines
net/nfc/core.c +5 -0
@@ -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/[email protected]

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

author maintainer
#2 lore raw
Quoted 107 lines · Johan Hovold
Quoted 94 lines · Greg Kroah-Hartman

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 <[email protected]> know about it.

From 20777bc57c346b6994f465e0d8261a7fbf213a09 Mon Sep 17 00:00:00 2001
From: Johan Hovold <[email protected]>
Date: Thu, 30 Mar 2017 12:15:35 +0200
Subject: NFC: fix broken device allocation

From: Johan Hovold <[email protected]>

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 <[email protected]>
---
 net/nfc/core.c     |   31 ++++++++++++++++++-------------
 net/nfc/nci/core.c |    3 +--
 2 files changed, 19 insertions(+), 15 deletions(-)
net/nfc/core.c +12 -0
@@ -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;

net/nfc/core.c struct nfc_dev *nfc_allocate_device(stru · line 1109

Quoted 8 lines
net/nfc/core.c +5 -0
@@ -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?):

Quoted 5 lines
	lkml.kernel.org/r/[email protected]

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