Commit 5ab894ae authored by Jarkko Nikula's avatar Jarkko Nikula Committed by Rafael J. Wysocki

device property: Track owner device of device property

Deletion of subdevice will remove device properties associated to parent
when they share the same firmware node after commit 478573c9 (driver
core: Don't leak secondary fwnode on device removal).  This was observed
with a driver adding subdevice that driver wasn't able to read device
properties after rmmod/modprobe cycle.

Consider the lifecycle of it:

parent device registration
	ACPI_COMPANION_SET()
	device_add_properties()
		pset_copy_set()
		set_secondary_fwnode(dev, &p->fwnode)
	device_add()

parent probe
	read device properties
	ACPI_COMPANION_SET(subdevice, ACPI_COMPANION(parent))
	device_add(subdevice)

parent remove
	device_del(subdevice)
		device_remove_properties()
			set_secondary_fwnode(dev, NULL);
			pset_free()

Parent device will have its primary firmware node pointing to an ACPI
node and secondary firmware node point to device properties.

ACPI_COMPANION_SET() call in parent probe will set the subdevice's
firmware node to point to the same 'struct fwnode_handle' and the
associated secondary firmware node, i.e. the device properties as the
parent.

When subdevice is deleted in parent remove that will remove those
device properties and attempt to read device properties in next
parent probe call will fail.

Fix this by tracking the owner device of device properties and delete
them only when owner device is being deleted.

Fixes: 478573c9 (driver core: Don't leak secondary fwnode on device removal)
Cc: 4.9+ <stable@vger.kernel.org> # 4.9+
Signed-off-by: default avatarJarkko Nikula <jarkko.nikula@linux.intel.com>
Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
parent 8a5776a5
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include <linux/phy.h> #include <linux/phy.h>
struct property_set { struct property_set {
struct device *dev;
struct fwnode_handle fwnode; struct fwnode_handle fwnode;
const struct property_entry *properties; const struct property_entry *properties;
}; };
...@@ -891,6 +892,7 @@ static struct property_set *pset_copy_set(const struct property_set *pset) ...@@ -891,6 +892,7 @@ static struct property_set *pset_copy_set(const struct property_set *pset)
void device_remove_properties(struct device *dev) void device_remove_properties(struct device *dev)
{ {
struct fwnode_handle *fwnode; struct fwnode_handle *fwnode;
struct property_set *pset;
fwnode = dev_fwnode(dev); fwnode = dev_fwnode(dev);
if (!fwnode) if (!fwnode)
...@@ -900,16 +902,16 @@ void device_remove_properties(struct device *dev) ...@@ -900,16 +902,16 @@ void device_remove_properties(struct device *dev)
* the pset. If there is no real firmware node (ACPI/DT) primary * the pset. If there is no real firmware node (ACPI/DT) primary
* will hold the pset. * will hold the pset.
*/ */
if (is_pset_node(fwnode)) { pset = to_pset_node(fwnode);
if (pset) {
set_primary_fwnode(dev, NULL); set_primary_fwnode(dev, NULL);
pset_free_set(to_pset_node(fwnode));
} else { } else {
fwnode = fwnode->secondary; pset = to_pset_node(fwnode->secondary);
if (!IS_ERR(fwnode) && is_pset_node(fwnode)) { if (pset && dev == pset->dev)
set_secondary_fwnode(dev, NULL); set_secondary_fwnode(dev, NULL);
pset_free_set(to_pset_node(fwnode));
}
} }
if (pset && dev == pset->dev)
pset_free_set(pset);
} }
EXPORT_SYMBOL_GPL(device_remove_properties); EXPORT_SYMBOL_GPL(device_remove_properties);
...@@ -938,6 +940,7 @@ int device_add_properties(struct device *dev, ...@@ -938,6 +940,7 @@ int device_add_properties(struct device *dev,
p->fwnode.ops = &pset_fwnode_ops; p->fwnode.ops = &pset_fwnode_ops;
set_secondary_fwnode(dev, &p->fwnode); set_secondary_fwnode(dev, &p->fwnode);
p->dev = dev;
return 0; return 0;
} }
EXPORT_SYMBOL_GPL(device_add_properties); EXPORT_SYMBOL_GPL(device_add_properties);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment