driver core/USB/thermal: fix device-tree node reuse

This series fixes a few issues related to device-tree node reuse.

It is fairly common for drivers to reuse the device-tree node of a parent (or other ancestor) device when creating class or bus devices (e.g. gpio chips, i2c adapters, iio chips, spi masters, serdev, phys, usb root hubs). But reusing a device-tree node may cause problems *if* the new device is later probed as for example driver core would currently attempt to reinitialise an already active associated pinmux configuration.

[ NB: For most examples above this is currently not a problem as the devices reusing a node are never probed. ]

Other potential issues include the platform-bus code unconditionally dropping the device-tree-node reference in its device destructor, reinitialisation of other bus-managed resources such as clocks, and (possibly) the recently added DMA-setup in driver core.

Instead of having drivers try to work around this (as is currently done for USB root hubs), we can allow devices to reuse a device-tree node by setting a flag in their struct device that can be used by core, bus and driver code to avoid resources from being over-allocated.

The first two patches fix a device-tree node reference leak for non-root-hub devices in USB. These were submitted yesterday to the USB list, but are included here for completeness.

The third and fourth patches add a helper that can be used when reusing a device-tree node of another device and that specifically sets a new of_node_reused flag which is then used by driver core to skip the automatic pinctrl configuration during probe.

The fifth patch removes the pinctrl over-allocation workaround from USB core, which also had some undesirable side-effects.

The final two patches fix a device-tree node imbalance and use-after-free in an thermal driver, where a platform device was reusing the device-tree node of its parent mfd during probe. This would also prevent the child device from being reprobed (e.g. due to probe deferral) if the parent node defines a pinctrl configuration.

Note that this series is against 4.12-rc3.

For reference, here is a list of relevant commits leading up to the current situation:

- [2013-01-22] ab78029ecc34 ("drivers/pinctrl: grab default handles from device core")
- [2016-02-19] 69bec7259853 ("USB: core: let USB device know device node")
- [2016-04-25] dc5878abf49c ("usb: core: move root hub's device node assignment after it is added to bus")
- [2016-10-06] 51fa91475e43 ("usb/core: Added devspec sysfs entry for devices behind the usb hub")
- [2017-03-13] a8c06e407ef9 ("usb: separate out sysdev pointer from usb_bus")
- [2017-04-10] 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices")

Greg, I'm not sure how best to handle merging this, but I guess all could go in through driver-core-next to make handling dependencies easier even if patches 1,2 and 6 could otherwise be considered for 4.12-rc.

Johan

Changes in v2
- update kernel doc comment (2/7)
- add missing kernel doc comment (3/7)
- add Peter's and Linus's acks (1/7 and 4/7)

Johan Hovold (7):
USB: core: fix device node leak
USB: of: document reference taken by child-lookup helper driver core: add helper to reuse a device-tree node driver core: fix automatic pinctrl management
USB: of: fix root-hub device-tree node handling
thermal: max77620: fix device-node reference imbalance
thermal: max77620: fix pinmux conflict on reprobe

 drivers/base/core.c                | 16 ++++++++++++++++
 drivers/base/pinctrl.c             |  3 +++
 drivers/thermal/max77620_thermal.c |  8 ++++++--
 drivers/usb/core/hcd.c             |  2 --
 drivers/usb/core/of.c              |  3 ++-
 drivers/usb/core/usb.c             |  2 ++
 include/linux/device.h             |  4 ++++
 7 files changed, 33 insertions(+), 5 deletions(-)

2.13.0

Make sure to release any OF device-node reference taken when creating the USB device.

Note that we currently do not hold a reference to the root hub device-tree node (i.e. the parent controller node).

---
 drivers/usb/core/usb.c | 2 ++
 1 file changed, 2 insertions(+)

index 28b053cacc90..62e1906bb2f3 100644
@@ -416,6 +416,8 @@ static void usb_release_dev(struct device *dev)
 
 	usb_destroy_configuration(udev);
 	usb_release_bos_descriptor(udev);
+	if (udev->parent)
+		of_node_put(dev->of_node);
 	usb_put_hcd(hcd);
 	kfree(udev->product);
 	kfree(udev->manufacturer);

2.13.0

Document that the child-node lookup helper takes a reference to the device-tree node which needs to be dropped after use.

---
 drivers/usb/core/of.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index d563cbcf76cf..3863bb1ce8c5 100644
@@ -28,7 +28,8 @@
  *
  * Find the node from device tree according to its port number.
  *
- * Return: On success, a pointer to the device node, %NULL on failure.
+ * Return: A pointer to the node with incremented refcount if found, or
+ * %NULL otherwise.
  */
 struct device_node *usb_of_get_child_node(struct device_node *parent,
 					int portnum)

2.13.0

Add a helper function to be used when reusing the device-tree node of another device.

It is fairly common for drivers to reuse the device-tree node of a parent (or other ancestor) device when creating class or bus devices (e.g. gpio chips, i2c adapters, iio chips, spi masters, serdev, phys, usb root hubs). But reusing a device-tree node may cause problems if the new device is later probed as for example driver core would currently attempt to reinitialise an already active associated pinmux configuration.

Other potential issues include the platform-bus code unconditionally dropping the device-tree node reference in its device destructor, reinitialisation of other bus-managed resources such as clocks, and the recently added DMA-setup in driver core.

Note that for most examples above this is currently not an issue as the devices are never probed, but this is a problem for the USB bus which has recently gained device-tree support. This was discovered and worked-around in a rather ad-hoc fashion by commit dc5878abf49c ("usb:
core: move root hub's device node assignment after it is added to bus") by not setting the of_node pointer until after the root-hub device has been registered.

Instead we can allow devices to reuse a device-tree node by setting a flag in their struct device that can be used by core, bus and driver code to avoid resources from being over-allocated.

Note that the helper also grabs an extra reference to the device node, which specifically balances the unconditional put in the platform-device destructor.

---
 drivers/base/core.c    | 16 ++++++++++++++++
 include/linux/device.h |  4 ++++
 2 files changed, 20 insertions(+)

index bbecaf9293be..c3064ff09af5 100644
@@ -2884,3 +2884,19 @@ void set_secondary_fwnode(struct device *dev, struct fwnode_handle *fwnode)
 	else
 		dev->fwnode = fwnode;
 }
+
+/**
+ * device_set_of_node_from_dev - reuse device-tree node of another device
+ * @dev: device whose device-tree node is being set
+ * @dev2: device whose device-tree node is being reused
+ *
+ * Takes another reference to the new device-tree node after first dropping
+ * any reference held to the old node.
+ */
+void device_set_of_node_from_dev(struct device *dev, const struct device *dev2)
+{
+	of_node_put(dev->of_node);
+	dev->of_node = of_node_get(dev2->of_node);
+	dev->of_node_reused = true;
+}
+EXPORT_SYMBOL_GPL(device_set_of_node_from_dev);

index 9ef518af5515..60ab00b13095 100644
@@ -879,6 +879,8 @@ struct dev_links_info {
  *
  * @offline_disabled: If set, the device is permanently online.
  * @offline:	Set after successful invocation of bus type's .offline().
+ * @of_node_reused: Set if the device-tree node is shared with an ancestor
+ *              device.
  *
  * At the lowest level, every device in a Linux system is represented by an
  * instance of struct device. The device structure contains the information

@@ -966,6 +968,7 @@ struct device {
 
 	bool			offline_disabled:1;
 	bool			offline:1;
+	bool			of_node_reused:1;
 };
 
 static inline struct device *kobj_to_dev(struct kobject *kobj)

@@ -1144,6 +1147,7 @@ extern int device_offline(struct device *dev);
 extern int device_online(struct device *dev);
 extern void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode);
 extern void set_secondary_fwnode(struct device *dev, struct fwnode_handle *fwnode);
+void device_set_of_node_from_dev(struct device *dev, const struct device *dev2);
 
 static inline int dev_num_vf(struct device *dev)
 {

2.13.0

Commit ab78029ecc34 ("drivers/pinctrl: grab default handles from device core") added automatic pin-control management to driver core by looking up and setting any default pinctrl state found in device tree while a device is being probed.

This obviously runs into problems as soon as device-tree nodes are reused for child devices which are later also probed as pins would already have been claimed by the ancestor device.

For example if a USB host controller claims a pin, its root hub would consequently fail to probe when its device-tree node is set to the node of the controller:

pinctrl-single 48002030.pinmux: pin PIN204 already requested by 48064800.ehci; cannot claim for usb1 pinctrl-single 48002030.pinmux: pin-204 (usb1) status -22
pinctrl-single 48002030.pinmux: could not request pin 204 (PIN204) from group usb_dbg_pins on device pinctrl-single usb usb1: Error applying setting, reverse things back
usb: probe of usb1 failed with error -22

Fix this by checking the new of_node_reused flag and skipping automatic pinctrl configuration during probe if set.

Note that the flag is checked in driver core rather than in pinctrl (e.g. in pinctrl_dt_to_map()) which would specifically have prevented intentional use of a parent's pinctrl properties by a child device (should such a need ever arise).

---
 drivers/base/pinctrl.c | 3 +++
 1 file changed, 3 insertions(+)

index 5917b4b5fb99..eb929dd6ef1e 100644
@@ -23,6 +23,9 @@ int pinctrl_bind_pins(struct device *dev)
 {
 	int ret;
 
+	if (dev->of_node_reused)
+		return 0;
+
 	dev->pins = devm_kzalloc(dev, sizeof(*(dev->pins)), GFP_KERNEL);
 	if (!dev->pins)
 		return -ENOMEM;

2.13.0

In an attempt to work around a pinmux over-allocation issue in driver core, commit dc5878abf49c ("usb: core: move root hub's device node assignment after it is added to bus") moved the device-tree node assignment until after the root hub had been registered.

This not only makes the device-tree node unavailable to the usb driver during probe, but also prevents the of_node from being linked to in sysfs and causes a race with user-space for the (recently added) devspec attribute.

Use the new device_set_of_node_from_dev() helper to reuse the node of the sysdev device, something which now prevents driver core from trying to reclaim any pinctrl pins during probe.

---
 drivers/usb/core/hcd.c | 2 --
 drivers/usb/core/usb.c | 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

index 5dea98358c05..2cff59e9c268 100644
@@ -1076,7 +1076,6 @@ static void usb_deregister_bus (struct usb_bus *bus)
 static int register_root_hub(struct usb_hcd *hcd)
 {
 	struct device *parent_dev = hcd->self.controller;
-	struct device *sysdev = hcd->self.sysdev;
 	struct usb_device *usb_dev = hcd->self.root_hub;
 	const int devnum = 1;
 	int retval;

@@ -1123,7 +1122,6 @@ static int register_root_hub(struct usb_hcd *hcd)
 		/* Did the HC die before the root hub was registered? */
 		if (HCD_DEAD(hcd))
 			usb_hc_died (hcd);	/* This time clean up */
-		usb_dev->dev.of_node = sysdev->of_node;
 	}
 	mutex_unlock(&usb_bus_idr_lock);
 

index 62e1906bb2f3..17681d5638ac 100644
@@ -416,8 +416,7 @@ static void usb_release_dev(struct device *dev)
 
 	usb_destroy_configuration(udev);
 	usb_release_bos_descriptor(udev);
-	if (udev->parent)
-		of_node_put(dev->of_node);
+	of_node_put(dev->of_node);
 	usb_put_hcd(hcd);
 	kfree(udev->product);
 	kfree(udev->manufacturer);

@@ -616,6 +615,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,
 		dev->route = 0;
 
 		dev->dev.parent = bus->controller;
+		device_set_of_node_from_dev(&dev->dev, bus->sysdev);
 		dev_set_name(&dev->dev, "usb%d", bus->busnum);
 		root_hub = 1;
 	} else {

2.13.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

The thermal child device reuses the parent MFD-device device-tree node when registering a thermal zone, but did not take a reference to the node.

This leads to a reference imbalance, and potential use-after-free, when the node reference is dropped by the platform-bus device destructor (once for the child and later again for the parent).

Fix this by dropping any reference already held to a device-tree node and getting a reference to the parent's node which will be balanced on reprobe or on platform-device release, whichever comes first.

Note that simply clearing the of_node pointer on probe errors and on driver unbind would not allow the use of device-managed resources as specifically thermal_zone_of_sensor_unregister() claims that a valid device-tree node pointer is needed during deregistration (even if it currently does not seem to use it).

---
 drivers/thermal/max77620_thermal.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

index e9a1fe342760..71d35f3c9215 100644
@@ -104,8 +104,6 @@ static int max77620_thermal_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	pdev->dev.of_node = pdev->dev.parent->of_node;
-
 	mtherm->dev = &pdev->dev;
 	mtherm->rmap = dev_get_regmap(pdev->dev.parent, NULL);
 	if (!mtherm->rmap) {

@@ -113,6 +111,14 @@ static int max77620_thermal_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	/*
+	 * Drop any current reference to a device-tree node and get a
+	 * reference to the parent's node which will be balanced on reprobe or
+	 * on platform-device release.
+	 */
+	of_node_put(pdev->dev.of_node);
+	pdev->dev.of_node = of_node_get(pdev->dev.parent->of_node);
+
 	mtherm->tz_device = devm_thermal_zone_of_sensor_register(&pdev->dev, 0,
 				mtherm, &max77620_thermal_ops);
 	if (IS_ERR(mtherm->tz_device)) {

2.13.0

Use the new helper for reusing a device-tree node of another device instead of managing the node references explicitly.

This also makes sure that the new of_node_reuse flag is set if the device is ever reprobed, something which specifically now avoids driver core from attempting to claim any pinmux resources already claimed by the parent device.

---
 drivers/thermal/max77620_thermal.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

index 71d35f3c9215..159bbcee8821 100644
@@ -112,12 +112,10 @@ static int max77620_thermal_probe(struct platform_device *pdev)
 	}
 
 	/*
-	 * Drop any current reference to a device-tree node and get a
-	 * reference to the parent's node which will be balanced on reprobe or
-	 * on platform-device release.
+	 * The reference taken to the parent's node which will be balanced on
+	 * reprobe or on platform-device release.
 	 */
-	of_node_put(pdev->dev.of_node);
-	pdev->dev.of_node = of_node_get(pdev->dev.parent->of_node);
+	device_set_of_node_from_dev(&pdev->dev, pdev->dev.parent);
 
 	mtherm->tz_device = devm_thermal_zone_of_sensor_register(&pdev->dev, 0,
 				mtherm, &max77620_thermal_ops);

2.13.0