driver core: generalize driver_override in struct device
PATCH 1/2 driver core: generalize driver_override in struct device
From: Danilo Krummrich <[email protected]>
[ Upstream commit cb3d1049f4ea77d5ad93f17d8ac1f2ed4da70501 ]
Currently, there are 12 busses (including platform and PCI) that duplicate the driver_override logic for their individual devices.
All of them seem to be prone to the bug described in [1].
While this could be solved for every bus individually using a separate lock, solving this in the driver-core generically results in less (and cleaner) changes overall.
Thus, move driver_override to struct device, provide corresponding accessors for busses and handle locking with a separate lock internally.
In particular, add device_set_driver_override(), device_has_driver_override(), device_match_driver_override() and generalize the sysfs store() and show() callbacks via a driver_override feature flag in struct bus_type.
Until all busses have migrated, keep driver_set_override() in place.
Note that we can't use the device lock for the reasons described in [2].
[ Use dev->bus instead of sp->bus for consistency; fix commit message to
refer to the struct bus_type's driver_override feature flag. - Danilo ]
Stable-dep-of: 2b38efc05bf7 ("driver core: platform: use generic driver_override infrastructure")
--- drivers/base/bus.c | 43 ++++++++++++++++++++++++++- drivers/base/core.c | 2 ++ drivers/base/dd.c | 60 ++++++++++++++++++++++++++++++++++++++ include/linux/device.h | 54 ++++++++++++++++++++++++++++++++++ include/linux/device/bus.h | 4 +++ 5 files changed, 162 insertions(+), 1 deletion(-)
drivers/base/bus.c +42 -1
@@ -463,6 +463,36 @@ int bus_for_each_drv(const struct bus_type *bus, struct device_driver *start, }EXPORT_SYMBOL_GPL (bus_for_each_drv); +static ssize_t driver_override_store (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t count) +{ +int ret; + + ret =__device_set_driver_override (dev, buf, count); +if (ret) +return ret; + +return count; +} + +static ssize_t driver_override_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +guard (spinlock)(&dev->driver_override.lock); +return sysfs_emit (buf,"%s\n" , dev->driver_override.name); +} +static DEVICE_ATTR_RW (driver_override); + +static struct attribute *driver_override_dev_attrs[] = { + &dev_attr_driver_override.attr, +NULL , +}; + +static const struct attribute_group driver_override_dev_group = { + .attrs = driver_override_dev_attrs, +}; +/** * bus_add_device - add device to bus * @dev: device being added
@@ -496,9 +526,15 @@ int bus_add_device(struct device *dev)if (error) goto out_put; +if (dev->bus->driver_override) { + error =device_add_group (dev, &driver_override_dev_group); +if (error) + goto out_groups; + } + error =sysfs_create_link (&sp->devices_kset->kobj, &dev->kobj,dev_name (dev));if (error) - goto out_groups; + goto out_override; error =sysfs_create_link (&dev->kobj, &sp->subsys.kobj,"subsystem" );if (error)
@@ -509,6 +545,9 @@ int bus_add_device(struct device *dev) out_subsys:sysfs_remove_link (&sp->devices_kset->kobj,dev_name (dev)); +out_override: +if (dev->bus->driver_override) +device_remove_group (dev, &driver_override_dev_group); out_groups:device_remove_groups (dev, sp->bus->dev_groups); out_put:
@@ -567,6 +606,8 @@ void bus_remove_device(struct device *dev)sysfs_remove_link (&dev->kobj,"subsystem" );sysfs_remove_link (&sp->devices_kset->kobj,dev_name (dev)); +if (dev->bus->driver_override) +device_remove_group (dev, &driver_override_dev_group);device_remove_groups (dev, dev->bus->dev_groups);if (klist_node_attached (&dev->p->knode_bus))klist_del (&dev->p->knode_bus);
drivers/base/core.c +2 -0
@@ -2505,6 +2505,7 @@ static void device_release(struct kobject *kobj)devres_release_all (dev);kfree (dev->dma_range_map); +kfree (dev->driver_override.name);if (dev->release) dev->release (dev);
@@ -3153,6 +3154,7 @@ void device_initialize(struct device *dev)kobject_init (&dev->kobj, &device_ktype);INIT_LIST_HEAD (&dev->dma_pools);mutex_init (&dev->mutex); +spin_lock_init (&dev->driver_override.lock);lockdep_set_novalidate_class (&dev->mutex);spin_lock_init (&dev->devres_lock);INIT_LIST_HEAD (&dev->devres_head);
drivers/base/dd.c +60 -0
66 lines not on this page · raw message
include/linux/device.h +54 -0
72 lines not on this page · raw message
include/linux/device/bus.h +4 -0
@@ -65,6 +65,9 @@ struct fwnode_handle;* @iommu_ops: IOMMU specific operations for this bus, used to attach IOMMU * driver implementations to a bus and allow the driver to do * bus-specific setup +* @driver_override: Set to true if this bus supports the driver_override +* mechanism, which allows userspace to force a specific +* driver to bind to a device via a sysfs attribute. * @need_parent_lock: When probing or removing a device on this bus, the * device core should lock the device's parent. *
@@ -106,6 +109,7 @@ struct bus_type {const struct iommu_ops *iommu_ops; +bool driver_override;bool need_parent_lock; };
Signature · 12 lines
2.47.3 Amazon Web Services Development Center Germany GmbH Tamara-Danz-Str. 13 10243 Berlin Geschaeftsfuehrung: Christof Hellmis, Andreas Stieger Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B Sitz: Berlin Ust-ID: DE 365 538 597
PATCH 2/2 driver core: platform: use generic driver_override infrastructure
From: Danilo Krummrich <[email protected]>
[ Upstream commit 2b38efc05bf7a8568ec74bfffea0f5cfa62bc01d ]
When a driver is probed through __driver_attach(), the bus' match() callback is called without the device lock held, thus accessing the driver_override field without a lock, which can cause a UAF.
Fix this by using the driver-core driver_override infrastructure taking care of proper locking internally.
Note that calling match() from __driver_attach() without the device lock held is intentional. [1]
--- drivers/base/platform.c | 37 +++++---------------------------- drivers/bus/simple-pm-bus.c | 4 ++-- drivers/clk/imx/clk-scu.c | 3 +-- drivers/slimbus/qcom-ngd-ctrl.c | 6 ++---- include/linux/platform_device.h | 5 ----- sound/soc/samsung/i2s.c | 6 +++--- 6 files changed, 13 insertions(+), 48 deletions(-)
drivers/base/platform.c +5 -32
@@ -561,7 +561,6 @@ static void platform_device_release(struct device *dev)kfree (pa->pdev.dev.platform_data);kfree (pa->pdev.mfd_cell);kfree (pa->pdev.resource); -kfree (pa->pdev.driver_override);kfree (pa); }
@@ -1265,38 +1264,9 @@ static ssize_t numa_node_show(struct device *dev, }static DEVICE_ATTR_RO (numa_node); -static ssize_t driver_override_show (struct device *dev, -struct device_attribute *attr,char *buf) -{ -struct platform_device *pdev =to_platform_device (dev); -ssize_t len; - -device_lock (dev); - len =sysfs_emit (buf,"%s\n" , pdev->driver_override); -device_unlock (dev); - -return len; -} - -static ssize_t driver_override_store (struct device *dev, -struct device_attribute *attr, -const char *buf,size_t count) -{ -struct platform_device *pdev =to_platform_device (dev); -int ret; - - ret =driver_set_override (dev, &pdev->driver_override, buf, count); -if (ret) -return ret; - -return count; -} -static DEVICE_ATTR_RW (driver_override); -static struct attribute *platform_dev_attrs[] = { &dev_attr_modalias.attr, &dev_attr_numa_node.attr, - &dev_attr_driver_override.attr,NULL , };
@@ -1336,10 +1306,12 @@ static int platform_match(struct device *dev, struct device_driver *drv) {struct platform_device *pdev =to_platform_device (dev);struct platform_driver *pdrv =to_platform_driver (drv); +int ret;/* When driver_override is set, only bind to the matching driver */ -if (pdev->driver_override) -return !strcmp (pdev->driver_override, drv->name); + ret =device_match_driver_override (dev, drv); +if (ret >=0 ) +return ret;/* Attempt an OF style match first */ if (of_driver_match_device (dev, drv))
@@ -1482,6 +1454,7 @@ static const struct dev_pm_ops platform_dev_pm_ops = {struct bus_type platform_bus_type = { .name ="platform" , .dev_groups = platform_dev_groups, + .driver_override = true, .match = platform_match, .uevent = platform_uevent, .probe = platform_probe,
drivers/bus/simple-pm-bus.c +2 -2
@@ -36,7 +36,7 @@ static int simple_pm_bus_probe(struct platform_device *pdev)* that's not listed in simple_pm_bus_of_match. We don't want to do any * of the simple-pm-bus tasks for these devices, so return early. */ -if (pdev->driver_override) +if (device_has_driver_override (&pdev->dev))return 0 ; match =of_match_device (dev->driver->of_match_table, dev);
@@ -78,7 +78,7 @@ static int simple_pm_bus_remove(struct platform_device *pdev) {const void *data =of_device_get_match_data (&pdev->dev); -if (pdev->driver_override || data) +if (device_has_driver_override (&pdev->dev) || data)return 0 ;dev_dbg (&pdev->dev,"%s\n" , __func__);
drivers/clk/imx/clk-scu.c +1 -2
@@ -700,8 +700,7 @@ struct clk_hw *imx_clk_scu_alloc_dev(const char *name,return ERR_PTR (ret); } - ret =driver_set_override (&pdev->dev, &pdev->driver_override, -"imx-scu-clk" ,strlen ("imx-scu-clk" )); + ret =device_set_driver_override (&pdev->dev,"imx-scu-clk" );if (ret) {platform_device_put (pdev);return ERR_PTR (ret);
drivers/slimbus/qcom-ngd-ctrl.c +2 -4
@@ -1537,10 +1537,8 @@ static int of_qcom_slim_ngd_register(struct device *parent, ngd->id = id; ngd->pdev->dev.parent = parent; - ret =driver_set_override (&ngd->pdev->dev, - &ngd->pdev->driver_override, -QCOM_SLIM_NGD_DRV_NAME , -strlen (QCOM_SLIM_NGD_DRV_NAME )); + ret =device_set_driver_override (&ngd->pdev->dev, +QCOM_SLIM_NGD_DRV_NAME );if (ret) {platform_device_put (ngd->pdev);kfree (ngd);
include/linux/platform_device.h +0 -5
@@ -31,11 +31,6 @@ struct platform_device {struct resource *resource;const struct platform_device_id *id_entry; -/* -* Driver name to force a match. Do not set directly, because core -* frees it. Use driver_set_override() to set or clear it. -*/ -const char *driver_override;/* MFD cell pointer */ struct mfd_cell *mfd_cell;
sound/soc/samsung/i2s.c +3 -3
@@ -1364,10 +1364,10 @@ static int i2s_create_secondary_device(struct samsung_i2s_priv *priv)if (!pdev_sec)return -ENOMEM ; - pdev_sec->driver_override =kstrdup ("samsung-i2s" ,GFP_KERNEL ); -if (!pdev_sec->driver_override) { + ret =device_set_driver_override (&pdev_sec->dev,"samsung-i2s" ); +if (ret) {platform_device_put (pdev_sec); -return -ENOMEM ; +return ret; } ret =platform_device_add (pdev_sec);
Signature · 12 lines
2.47.3 Amazon Web Services Development Center Germany GmbH Tamara-Danz-Str. 13 10243 Berlin Geschaeftsfuehrung: Christof Hellmis, Andreas Stieger Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B Sitz: Berlin Ust-ID: DE 365 538 597
Queued for 6.6, thanks.
Signature · 2 lines
Thanks, Sasha