treewide: Convert buses to use generic driver_override
Subsystems (21)
Danilo Krummrich
maintainercover treewide: Convert buses to use generic driver_override
This is the follow-up of the driver_override generalization in [1], converting the remaining 11 busses and removing the now-unused driver_set_override() helper.
All of them (except AP, which has a different race condition) are prone to the potential UAF described in [2], caused by accessing the driver_override field from their corresponding match() callback.
In order to address this, the generalized driver_override field in struct device is protected with a spinlock. The driver-core provides accessors, such as device_match_driver_override(), device_has_driver_override() and device_set_driver_override(), which all ensure proper locking internally.
Additionally, the driver-core provides a driver_override flag in struct bus_type, which, once enabled, automatically registers generic sysfs callbacks, allowing userspace to modify the driver_override field.
SPI and AP are a bit special; both print "\n" when driver_override is not set, whereas all other buses (and thus the driver-core) produce "(null)\n" in this case.
Hence, SPI and AP do not take advantage of the driver_override flag in struct bus_type; AP additionally maintains a counter in its custom sysfs store().
Technically, we could support a custom fallback string when driver_override is unset in struct bus_type, but only SPI would benefit from this, since AP has additional custom logic in store() anyways.
(I'm not sure if there are userspace programs that strictly rely on this; driverctl seems to check for both, but I rather not break some userspace tool I'm not aware of. :)
This series is based on v7.0-rc5 with no additional dependencies, hence those patches can be picked up by subsystems individually.
[1] https://lore.kernel.org/driver-core/[email protected]/
[2] https://bugzilla.kernel.org/show_bug.cgi?id=220789
[3] https://gitlab.com/driverctl/driverctl/-/blob/0.121/driverctl?ref_type=tags#L99
Danilo Krummrich (12):
amba: use generic driver_override infrastructure
bus: fsl-mc: use generic driver_override infrastructure
cdx: use generic driver_override infrastructure
hv: vmbus: use generic driver_override infrastructure
PCI: use generic driver_override infrastructure platform/wmi: use generic driver_override infrastructure
rpmsg: use generic driver_override infrastructure
vdpa: use generic driver_override infrastructure s390/cio: use generic driver_override infrastructure s390/ap: use generic driver_override infrastructure
spi: use generic driver_override infrastructure driver core: remove driver_set_override()
drivers/amba/bus.c | 37 +++------------ drivers/base/driver.c | 75 ------------------------------ drivers/bus/fsl-mc/fsl-mc-bus.c | 43 +++-------------- drivers/cdx/cdx.c | 40 ++-------------- drivers/hv/vmbus_drv.c | 36 ++------------ drivers/pci/pci-driver.c | 11 +++-- drivers/pci/pci-sysfs.c | 28 ----------- drivers/pci/probe.c | 1 - drivers/platform/wmi/core.c | 36 ++------------ drivers/rpmsg/qcom_glink_native.c | 2 - drivers/rpmsg/rpmsg_core.c | 43 +++-------------- drivers/rpmsg/virtio_rpmsg_bus.c | 1 - drivers/s390/cio/cio.h | 5 -- drivers/s390/cio/css.c | 34 ++------------ drivers/s390/crypto/ap_bus.c | 34 +++++++------- drivers/s390/crypto/ap_bus.h | 1 - drivers/s390/crypto/ap_queue.c | 24 +++------- drivers/spi/spi.c | 19 +++----- drivers/vdpa/vdpa.c | 48 ++----------------- drivers/vfio/fsl-mc/vfio_fsl_mc.c | 4 +- drivers/vfio/pci/vfio_pci_core.c | 5 +- drivers/xen/xen-pciback/pci_stub.c | 6 ++- include/linux/amba/bus.h | 5 -- include/linux/cdx/cdx_bus.h | 4 -- include/linux/device/driver.h | 2 - include/linux/fsl/mc.h | 4 -- include/linux/hyperv.h | 5 -- include/linux/pci.h | 6 --- include/linux/rpmsg.h | 4 -- include/linux/spi/spi.h | 5 -- include/linux/vdpa.h | 4 -- include/linux/wmi.h | 4 -- 32 files changed, 88 insertions(+), 488 deletions(-)
base-commit: c369299895a591d96745d6492d4888259b004a9e
Signature · 1 line
2.53.0
Danilo Krummrich
maintainerPATCH 1/12 amba: use generic driver_override infrastructure
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/amba/bus.c | 37 ++++++------------------------------- include/linux/amba/bus.h | 5 ----- 2 files changed, 6 insertions(+), 36 deletions(-)
drivers/amba/bus.c +6 -31
@@ -82,33 +82,6 @@ static void amba_put_disable_pclk(struct amba_device *pcdev) } -static ssize_t driver_override_show (struct device *_dev, -struct device_attribute *attr,char *buf) -{ -struct amba_device *dev =to_amba_device (_dev); -ssize_t len; - -device_lock (_dev); - len =sprintf (buf,"%s\n" , dev->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 amba_device *dev =to_amba_device (_dev); -int ret; - - ret =driver_set_override (_dev, &dev->driver_override, buf, count); -if (ret) -return ret; - -return count; -} -static DEVICE_ATTR_RW (driver_override); - #define amba_attr_func(name,fmt,arg...) \ static ssize_t name##_show(struct device *_dev, \ struct device_attribute *attr, char *buf) \
@@ -126,7 +99,6 @@ amba_attr_func(resource, "\t%016llx\t%016llx\t%016lx\n",static struct attribute *amba_dev_attrs[] = { &dev_attr_id.attr, &dev_attr_resource.attr, - &dev_attr_driver_override.attr,NULL , };ATTRIBUTE_GROUPS (amba_dev);
@@ -209,10 +181,11 @@ static int amba_match(struct device *dev, const struct device_driver *drv) {struct amba_device *pcdev =to_amba_device (dev);const struct amba_driver *pcdrv =to_amba_driver (drv); +int ret;mutex_lock (&pcdev->periphid_lock);if (!pcdev->periphid) { -int ret =amba_read_periphid (pcdev); + ret =amba_read_periphid (pcdev);/* * Returning any error other than -EPROBE_DEFER from bus match
@@ -230,8 +203,9 @@ static int amba_match(struct device *dev, const struct device_driver *drv)mutex_unlock (&pcdev->periphid_lock);/* When driver_override is set, only bind to the matching driver */ -if (pcdev->driver_override) -return !strcmp (pcdev->driver_override, drv->name); + ret =device_match_driver_override (dev, drv); +if (ret >=0 ) +return ret;return amba_lookup (pcdrv->id_table, pcdev) !=NULL ; }
@@ -436,6 +410,7 @@ static const struct dev_pm_ops amba_pm = {const struct bus_type amba_bustype = { .name ="amba" , .dev_groups = amba_dev_groups, + .driver_override = true, .match = amba_match, .uevent = amba_uevent, .probe = amba_probe,
include/linux/amba/bus.h +0 -5
@@ -71,11 +71,6 @@ struct amba_device {unsigned int cid;struct amba_cs_uci_id uci;unsigned int irq[AMBA_NR_IRQS ]; -/* -* 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; };struct amba_driver {
Signature · 1 line
2.53.0
Danilo Krummrich
maintainerPATCH 2/12 bus: fsl-mc: use generic driver_override infrastructure
In reply to Danilo KrummrichWhen 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/bus/fsl-mc/fsl-mc-bus.c | 43 +++++-------------------------- drivers/vfio/fsl-mc/vfio_fsl_mc.c | 4 +-- include/linux/fsl/mc.h | 4 --- 3 files changed, 8 insertions(+), 43 deletions(-)
drivers/bus/fsl-mc/fsl-mc-bus.c +7 -36
@@ -86,12 +86,16 @@ static int fsl_mc_bus_match(struct device *dev, const struct device_driver *drv)struct fsl_mc_device *mc_dev =to_fsl_mc_device (dev);const struct fsl_mc_driver *mc_drv =to_fsl_mc_driver (drv);bool found = false; +int ret;/* When driver_override is set, only bind to the matching driver */ -if (mc_dev->driver_override) { - found = !strcmp (mc_dev->driver_override, mc_drv->driver.name); + ret =device_match_driver_override (dev, drv); +if (ret >0 ) { + found = true; goto out; } +if (ret ==0 ) + goto out;if (!mc_drv->match_id_table) goto out;
@@ -210,39 +214,8 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr, }static DEVICE_ATTR_RO (modalias); -static ssize_t driver_override_store (struct device *dev, -struct device_attribute *attr, -const char *buf,size_t count) -{ -struct fsl_mc_device *mc_dev =to_fsl_mc_device (dev); -int ret; - -if (WARN_ON (dev->bus != &fsl_mc_bus_type)) -return -EINVAL ; - - ret =driver_set_override (dev, &mc_dev->driver_override, buf, count); -if (ret) -return ret; - -return count; -} - -static ssize_t driver_override_show (struct device *dev, -struct device_attribute *attr,char *buf) -{ -struct fsl_mc_device *mc_dev =to_fsl_mc_device (dev); -ssize_t len; - -device_lock (dev); - len =sysfs_emit (buf,"%s\n" , mc_dev->driver_override); -device_unlock (dev); -return len; -} -static DEVICE_ATTR_RW (driver_override); -static struct attribute *fsl_mc_dev_attrs[] = { &dev_attr_modalias.attr, - &dev_attr_driver_override.attr,NULL , };
@@ -345,6 +318,7 @@ ATTRIBUTE_GROUPS(fsl_mc_bus);const struct bus_type fsl_mc_bus_type = { .name ="fsl-mc" , + .driver_override = true, .match = fsl_mc_bus_match, .uevent = fsl_mc_bus_uevent, .probe = fsl_mc_probe,
@@ -910,9 +884,6 @@ static struct notifier_block fsl_mc_nb;*/ void fsl_mc_device_remove (struct fsl_mc_device *mc_dev) { -kfree (mc_dev->driver_override); - mc_dev->driver_override =NULL ; -/* * The device-specific remove callback will get invoked by device_del() */
drivers/vfio/fsl-mc/vfio_fsl_mc.c +1 -3
@@ -424,9 +424,7 @@ static int vfio_fsl_mc_bus_notifier(struct notifier_block *nb,if (action ==BUS_NOTIFY_ADD_DEVICE && vdev->mc_dev == mc_cont) { - mc_dev->driver_override =kasprintf (GFP_KERNEL ,"%s" , - vfio_fsl_mc_ops.name); -if (!mc_dev->driver_override) +if (device_set_driver_override (dev, vfio_fsl_mc_ops.name))dev_warn (dev,"VFIO_FSL_MC: Setting driver override for device in dprc %s failed\n" ,dev_name (&mc_cont->dev));else
include/linux/fsl/mc.h +0 -4
@@ -178,9 +178,6 @@ struct fsl_mc_obj_desc {* @regions: pointer to array of MMIO region entries * @irqs: pointer to array of pointers to interrupts allocated to this device * @resource: generic resource associated with this MC object device, if any. -* @driver_override: driver name to force a match; do not set directly, -* because core frees it; use driver_set_override() to -* set or clear it. * * Generic device object for MC object devices that are "attached" to a * MC bus.
@@ -214,7 +211,6 @@ struct fsl_mc_device {struct fsl_mc_device_irq **irqs;struct fsl_mc_resource *resource;struct device_link *consumer_link; -const char *driver_override; };#define to_fsl_mc_device (_dev) \
Signature · 1 line
2.53.0
Re: [PATCH 02/12] bus: fsl-mc: use generic driver_override infrastructure
Quoted 7 lines · Danilo Krummrich
When a driver is probed through
__driver_attach(), the bus'match()callback is called without the device lock held, thus accessing thedriver_overridefield without a lock, which can cause a UAF.Fix this by using the driver-core
driver_overrideinfrastructure taking care of proper locking internally.
Quoted 8 lines
Note that calling
match()from__driver_attach()without the device lock held is intentional. [1]Reported-by Gui-Dong Han <[email protected]>Fixes 1f86a00c1159 ("bus/fsl-mc: add support for 'driver_override' in the mc-bus")Signed-off-by Danilo Krummrich <[email protected]>
Re: [PATCH 02/12] bus: fsl-mc: use generic driver_override infrastructure
Le 25/03/2026 à 13:01, Ioana Ciornei a écrit :
Quoted 11 lines · Danilo Krummrich
Quoted 10 lines
When a driver is probed through
__driver_attach(), the bus'match()callback is called without the device lock held, thus accessing thedriver_overridefield without a lock, which can cause a UAF.Fix this by using the driver-core
driver_overrideinfrastructure taking care of proper locking internally.Note that calling
match()from__driver_attach()without the device lock held is intentional. [1]
Quoted 9 lines
Quoted 5 lines
Reported-by Gui-Dong Han <[email protected]>Fixes 1f86a00c1159 ("bus/fsl-mc: add support for 'driver_override' in the mc-bus")Signed-off-by Danilo Krummrich <[email protected]>Tested-by Ioana Ciornei <[email protected]>Signed-off-by Ioana Ciornei <[email protected]>
Applied, thanks
Christophe Leroy (CS GROUP)
maintainerRe: [PATCH 02/12] bus: fsl-mc: use generic driver_override infrastructure
Le 28/03/2026 à 13:10, Christophe Leroy (CS GROUP) a écrit :
Quoted 20 lines
Le 25/03/2026 à 13:01, Ioana Ciornei a écrit :
Quoted 17 lines · Danilo Krummrich
Quoted 15 lines
When a driver is probed through
__driver_attach(), the bus'match()callback is called without the device lock held, thus accessing thedriver_overridefield without a lock, which can cause a UAF.Fix this by using the driver-core
driver_overrideinfrastructure taking care of proper locking internally.Note that calling
match()from__driver_attach()without the device lock held is intentional. [1]Link https://eur01.safelinks.protection.outlook.com/? url=https%3A%2F%2Flore.kernel.org%2Fdriver- core%2FDGRGTIRHA62X.3RY09D9SOK77P%40kernel.org%2F&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7C4b9262ddecdd4ce29f9808de8a66485e%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C639100369055903282%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=%2BRfjlUkq7oWV%2F0v2S2B%2BEuxCY%2FLRQv6qHiEWiupd6kc%3D&reserved=0 [1]Reported-by Gui-Dong Han <[email protected]>Closes https://eur01.safelinks.protection.outlook.com/? url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D220789&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7C4b9262ddecdd4ce29f9808de8a66485e%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C639100369055936232%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=XL1K1ICiygOZnlvDUbQFe192KnLsBQms0HFNGCuyz%2Fw%3D&reserved=0Fixes 1f86a00c1159 ("bus/fsl-mc: add support for 'driver_override' in the mc-bus")Signed-off-by Danilo Krummrich <[email protected]>
Quoted 6 lines
Quoted 3 lines
Tested-by Ioana Ciornei <[email protected]>Signed-off-by Ioana Ciornei <[email protected]>Applied, thanks
Have to drop it for now, build fails:
CALL scripts/checksyscalls.sh
CC drivers/bus/fsl-mc/fsl-mc-bus.odrivers/bus/fsl-mc/fsl-mc-bus.c: In function 'fsl_mc_bus_match':drivers/bus/fsl-mc/fsl-mc-bus.c:92:15: error: implicit declaration of function 'device_match_driver_override' [-Werror=implicit-function-declaration]
92 | ret = device_match_driver_override(dev, drv);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/bus/fsl-mc/fsl-mc-bus.c: At top level:
drivers/bus/fsl-mc/fsl-mc-bus.c:321:10: error: 'const struct bus_type' has no member named 'driver_override'
321 | .driver_override = true,
| ^~~~~~~~~~~~~~~drivers/bus/fsl-mc/fsl-mc-bus.c:321:28: warning: initialization of 'const char *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
321 | .driver_override = true,
| ^~~~drivers/bus/fsl-mc/fsl-mc-bus.c:321:28: note: (near initialization for 'fsl_mc_bus_type.dev_name')
cc1: some warnings being treated as errors
make[5]: *** [scripts/Makefile.build:289: drivers/bus/fsl-mc/fsl-mc-bus.o] Error 1
make[4]: *** [scripts/Makefile.build:546: drivers/bus/fsl-mc] Error 2
make[3]: *** [scripts/Makefile.build:546: drivers/bus] Error 2
make[2]: *** [scripts/Makefile.build:546: drivers] Error 2
make[1]: *** [/home/chleroy/linux-powerpc/Makefile:2101: .] Error 2
make: *** [Makefile:248: __sub-make] Error 2
Christophe
Danilo Krummrich
maintainerPATCH 3/12 cdx: use generic driver_override infrastructure
In reply to Danilo KrummrichWhen 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/cdx/cdx.c | 40 +++++-------------------------------- include/linux/cdx/cdx_bus.h | 4 ---- 2 files changed, 5 insertions(+), 39 deletions(-)
drivers/cdx/cdx.c +5 -35
@@ -156,8 +156,6 @@ static int cdx_unregister_device(struct device *dev, }else {cdx_destroy_res_attr (cdx_dev,MAX_CDX_DEV_RESOURCES );debugfs_remove_recursive (cdx_dev->debugfs_dir); -kfree (cdx_dev->driver_override); - cdx_dev->driver_override =NULL ; }/*
@@ -268,6 +266,7 @@ static int cdx_bus_match(struct device *dev, const struct device_driver *drv)const struct cdx_driver *cdx_drv =to_cdx_driver (drv);const struct cdx_device_id *found_id =NULL ;const struct cdx_device_id *ids; +int ret;if (cdx_dev->is_bus)return false;
@@ -275,7 +274,8 @@ static int cdx_bus_match(struct device *dev, const struct device_driver *drv) ids = cdx_drv->match_id_table;/* When driver_override is set, only bind to the matching driver */ -if (cdx_dev->driver_override &&strcmp (cdx_dev->driver_override, drv->name)) + ret =device_match_driver_override (dev, drv); +if (ret ==0 )return false; found_id =cdx_match_id (ids, cdx_dev);
@@ -289,7 +289,7 @@ static int cdx_bus_match(struct device *dev, const struct device_driver *drv)*/ if (!found_id->override_only)return true; -if (cdx_dev->driver_override) +if (ret >0 )return true; ids = found_id +1 ;
@@ -453,36 +453,6 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr, }static DEVICE_ATTR_RO (modalias); -static ssize_t driver_override_store (struct device *dev, -struct device_attribute *attr, -const char *buf,size_t count) -{ -struct cdx_device *cdx_dev =to_cdx_device (dev); -int ret; - -if (WARN_ON (dev->bus != &cdx_bus_type)) -return -EINVAL ; - - ret =driver_set_override (dev, &cdx_dev->driver_override, buf, count); -if (ret) -return ret; - -return count; -} - -static ssize_t driver_override_show (struct device *dev, -struct device_attribute *attr,char *buf) -{ -struct cdx_device *cdx_dev =to_cdx_device (dev); -ssize_t len; - -device_lock (dev); - len =sysfs_emit (buf,"%s\n" , cdx_dev->driver_override); -device_unlock (dev); -return len; -} -static DEVICE_ATTR_RW (driver_override); -static ssize_t enable_store (struct device *dev,struct device_attribute *attr,const char *buf,size_t count) {
@@ -552,7 +522,6 @@ static struct attribute *cdx_dev_attrs[] = { &dev_attr_class.attr, &dev_attr_revision.attr, &dev_attr_modalias.attr, - &dev_attr_driver_override.attr, NULL, };
@@ -646,6 +615,7 @@ ATTRIBUTE_GROUPS(cdx_bus);const struct bus_type cdx_bus_type = { .name ="cdx" , + .driver_override = true, .match = cdx_bus_match, .probe = cdx_probe, .remove = cdx_remove,
include/linux/cdx/cdx_bus.h +0 -4
@@ -137,9 +137,6 @@ struct cdx_controller {* @enabled: is this bus enabled * @msi_dev_id: MSI Device ID associated with CDX device * @num_msi: Number of MSI's supported by the device -* @driver_override: driver name to force a match; do not set directly, -* because core frees it; use driver_set_override() to -* set or clear it. * @irqchip_lock: lock to synchronize irq/msi configuration * @msi_write_pending: MSI write pending for this device */
@@ -165,7 +162,6 @@ struct cdx_device {bool enabled;u32 msi_dev_id;u32 num_msi; -const char *driver_override;struct mutex irqchip_lock;bool msi_write_pending; };
Signature · 1 line
2.53.0
Danilo Krummrich
maintainerPATCH 4/12 hv: vmbus: use generic driver_override infrastructure
In reply to Danilo KrummrichWhen 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/hv/vmbus_drv.c | 36 +++++------------------------------- include/linux/hyperv.h | 5 ----- 2 files changed, 5 insertions(+), 36 deletions(-)
drivers/hv/vmbus_drv.c +5 -31
@@ -541,34 +541,6 @@ static ssize_t device_show(struct device *dev, }static DEVICE_ATTR_RO (device); -static ssize_t driver_override_store (struct device *dev, -struct device_attribute *attr, -const char *buf,size_t count) -{ -struct hv_device *hv_dev =device_to_hv_device (dev); -int ret; - - ret =driver_set_override (dev, &hv_dev->driver_override, buf, count); -if (ret) -return ret; - -return count; -} - -static ssize_t driver_override_show (struct device *dev, -struct device_attribute *attr,char *buf) -{ -struct hv_device *hv_dev =device_to_hv_device (dev); -ssize_t len; - -device_lock (dev); - len =sysfs_emit (buf,"%s\n" , hv_dev->driver_override); -device_unlock (dev); - -return len; -} -static DEVICE_ATTR_RW (driver_override); -/* Set up per device attributes in /sys/bus/vmbus/devices/<bus device> */ static struct attribute *vmbus_dev_attrs[] = { &dev_attr_id.attr,
@@ -599,7 +571,6 @@ static struct attribute *vmbus_dev_attrs[] = { &dev_attr_channel_vp_mapping.attr, &dev_attr_vendor.attr, &dev_attr_device.attr, - &dev_attr_driver_override.attr, NULL, };
@@ -711,9 +682,11 @@ static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver * {const guid_t *guid = &dev->dev_type;const struct hv_vmbus_device_id *id; +int ret;/* When driver_override is set, only bind to the matching driver */ -if (dev->driver_override &&strcmp (dev->driver_override, drv->name)) + ret =device_match_driver_override (&dev->device, &drv->driver); +if (ret ==0 )return NULL ;/* Look at the dynamic ids first, before the static ones */
@@ -722,7 +695,7 @@ static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver * id =hv_vmbus_dev_match (drv->id_table, guid);/* driver_override will always match, send a dummy id */ -if (!id && dev->driver_override) +if (!id && ret >0 ) id = &vmbus_device_null;return id;
@@ -1024,6 +997,7 @@ static const struct dev_pm_ops vmbus_pm = {/* The one and only one */ static const struct bus_type hv_bus = { .name ="vmbus" , + .driver_override = true, .match = vmbus_match, .shutdown = vmbus_shutdown, .remove = vmbus_remove,
include/linux/hyperv.h +0 -5
@@ -1272,11 +1272,6 @@ struct hv_device {u16 device_id;struct device device; -/* -* 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;struct vmbus_channel *channel;struct kset *channels_kset;
Signature · 1 line
2.53.0
RE: [PATCH 04/12] hv: vmbus: use generic driver_override infrastructure
From: Danilo Krummrich <[email protected]> Sent: Monday, March 23, 2026 5:59 PM
Quoted 1 line
In the patch "Subject" line, the prefix for changes for vmbus_drv.c has historically been "Drivers: hv: vmbus:". It's a mouthful, but has been kept fairly consistent over time.
Quoted 9 lines
When a driver is probed through
__driver_attach(), the bus'match()callback is called without the device lock held, thus accessing thedriver_overridefield without a lock, which can cause a UAF.Fix this by using the driver-core
driver_overrideinfrastructure taking care of proper locking internally.Note that calling
match()from__driver_attach()without the device lock held is intentional. [1]
I've tested this patch in a Hyper-V VM with VMBus devices. Did a simple VMBus driver override, listed the overrides, and then removed the override. All the right things happened with driver binding, unbind, etc.
Modulo updates to the comments that I've noted below (and the patch Subject line mentioned above):
Quoted 15 lines
Reported-by Gui-Dong Han <[email protected]>Fixes d765edbb301c ("vmbus: add driver_override support")Signed-off-by Danilo Krummrich <[email protected]>--- drivers/hv/vmbus_drv.c | 36 +++++------------------------------- include/linux/hyperv.h | 5 ----- 2 files changed, 5 insertions(+), 36 deletions(-)drivers/hv/vmbus_drv.c +0
-0
[snip]
drivers/hv/vmbus_drv.c static const struct hv_vmbus_device_id · line 688
Quoted 9 lines
drivers/hv/vmbus_drv.c +1
-0@@ -711,9 +682,11 @@ static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver * { const guid_t *guid = &dev->dev_type; const struct hv_vmbus_device_id *id; + int ret; /* When driver_override is set, only bind to the matching driver */
This reference to "driver_override" in the comment was originally to the "driver_override" field in struct hv_device, which has now gone away. Better wording would be "If a driver override is set, only bind ...."
drivers/hv/vmbus_drv.c static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver * · line 697
Quoted 10 lines
drivers/hv/vmbus_drv.c +2
-1-if (dev->driver_override &&strcmp (dev->driver_override, drv->name)) + ret =device_match_driver_override (&dev->device, &drv->driver); +if (ret ==0 )return NULL ;drivers/hv/vmbus_drv.c +0
-0/* Look at the dynamic ids first, before the static ones */ @@ -722,7 +695,7 @@ static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver * id =hv_vmbus_dev_match (drv->id_table, guid);/* driver_override will always match, send a dummy id */
Again, the reference to "driver_override" no longer makes sense. The original comment is a bit opaque in its own way. Let me suggest this new wording:
If there's a matching driver override, this function should succeed. So return a dummy device ID if no matching ID is found.
Quoted 32 lines
drivers/hv/vmbus_drv.c +2
-1-if (!id && dev->driver_override) +if (!id && ret >0 ) id = &vmbus_device_null;return id;@@ -1024,6 +997,7 @@ static const struct dev_pm_ops vmbus_pm = {/* The one and only one */ static const struct bus_type hv_bus = { .name ="vmbus" , + .driver_override = true, .match = vmbus_match, .shutdown = vmbus_shutdown, .remove = vmbus_remove,include/linux/hyperv.h +0
-6@@ -1272,11 +1272,6 @@ struct hv_device {u16 device_id;struct device device; -/* -* 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;struct vmbus_channel *channel;struct kset *channels_kset; --2.53.0
Danilo Krummrich
maintainerPATCH v2 1/1 Drivers: hv: vmbus: use generic driver_override infrastructure
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]
---
Changes in v2:
- Change patch subject and comments according to Michael's suggestion.
--- drivers/hv/vmbus_drv.c | 43 ++++++++++-------------------------------- include/linux/hyperv.h | 5 ----- 2 files changed, 10 insertions(+), 38 deletions(-)
drivers/hv/vmbus_drv.c +10 -33
@@ -541,34 +541,6 @@ static ssize_t device_show(struct device *dev, }static DEVICE_ATTR_RO (device); -static ssize_t driver_override_store (struct device *dev, -struct device_attribute *attr, -const char *buf,size_t count) -{ -struct hv_device *hv_dev =device_to_hv_device (dev); -int ret; - - ret =driver_set_override (dev, &hv_dev->driver_override, buf, count); -if (ret) -return ret; - -return count; -} - -static ssize_t driver_override_show (struct device *dev, -struct device_attribute *attr,char *buf) -{ -struct hv_device *hv_dev =device_to_hv_device (dev); -ssize_t len; - -device_lock (dev); - len =sysfs_emit (buf,"%s\n" , hv_dev->driver_override); -device_unlock (dev); - -return len; -} -static DEVICE_ATTR_RW (driver_override); -/* Set up per device attributes in /sys/bus/vmbus/devices/<bus device> */ static struct attribute *vmbus_dev_attrs[] = { &dev_attr_id.attr,
@@ -599,7 +571,6 @@ static struct attribute *vmbus_dev_attrs[] = { &dev_attr_channel_vp_mapping.attr, &dev_attr_vendor.attr, &dev_attr_device.attr, - &dev_attr_driver_override.attr, NULL, };
@@ -711,9 +682,11 @@ static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver * {const guid_t *guid = &dev->dev_type;const struct hv_vmbus_device_id *id; +int ret; -/* When driver_override is set, only bind to the matching driver */ -if (dev->driver_override &&strcmp (dev->driver_override, drv->name)) +/* If a driver override is set, only bind to the matching driver */ + ret =device_match_driver_override (&dev->device, &drv->driver); +if (ret ==0 )return NULL ;/* Look at the dynamic ids first, before the static ones */
@@ -721,8 +694,11 @@ static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver *if (!id) id =hv_vmbus_dev_match (drv->id_table, guid); -/* driver_override will always match, send a dummy id */ -if (!id && dev->driver_override) +/* +* If there's a matching driver override, this function should succeed, +* thus return a dummy device ID if no matching ID is found. +*/ +if (!id && ret >0 ) id = &vmbus_device_null;return id;
@@ -1024,6 +1000,7 @@ static const struct dev_pm_ops vmbus_pm = {/* The one and only one */ static const struct bus_type hv_bus = { .name ="vmbus" , + .driver_override = true, .match = vmbus_match, .shutdown = vmbus_shutdown, .remove = vmbus_remove,
include/linux/hyperv.h +0 -5
@@ -1272,11 +1272,6 @@ struct hv_device {u16 device_id;struct device device; -/* -* 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;struct vmbus_channel *channel;struct kset *channels_kset;
Signature · 1 line
2.53.0
Danilo Krummrich
maintainerPATCH 5/12 PCI: use generic driver_override infrastructure
In reply to Danilo KrummrichWhen 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/pci/pci-driver.c | 11 +++++++---- drivers/pci/pci-sysfs.c | 28 ---------------------------- drivers/pci/probe.c | 1 - drivers/vfio/pci/vfio_pci_core.c | 5 ++--- drivers/xen/xen-pciback/pci_stub.c | 6 ++++-- include/linux/pci.h | 6 ------ 6 files changed, 13 insertions(+), 44 deletions(-)
drivers/pci/pci-driver.c +7 -4
@@ -138,9 +138,11 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv, {struct pci_dynid *dynid;const struct pci_device_id *found_id =NULL , *ids; +int ret;/* When driver_override is set, only bind to the matching driver */ -if (dev->driver_override &&strcmp (dev->driver_override, drv->name)) + ret =device_match_driver_override (&dev->dev, &drv->driver); +if (ret ==0 )return NULL ;/* Look at the dynamic ids first, before the static ones */
@@ -164,7 +166,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,* matching. */ if (found_id->override_only) { -if (dev->driver_override) +if (ret >0 )return found_id; }else {return found_id;
@@ -172,7 +174,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv, }/* driver_override will always match, send a dummy id */ -if (dev->driver_override) +if (ret >0 )return &pci_device_id_any;return NULL ; }
@@ -452,7 +454,7 @@ static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)static inline bool pci_device_can_probe (struct pci_dev *pdev) {return (!pdev->is_virtfn || pdev->physfn->sriov->drivers_autoprobe || - pdev->driver_override); +device_has_driver_override (&pdev->dev)); }#else static inline bool pci_device_can_probe (struct pci_dev *pdev)
@@ -1722,6 +1724,7 @@ static const struct cpumask *pci_device_irq_get_affinity(struct device *dev,const struct bus_type pci_bus_type = { .name ="pci" , + .driver_override = true, .match = pci_bus_match, .uevent = pci_uevent, .probe = pci_device_probe,
drivers/pci/pci-sysfs.c +0 -28
@@ -615,33 +615,6 @@ static ssize_t devspec_show(struct device *dev,static DEVICE_ATTR_RO (devspec);#endif -static ssize_t driver_override_store (struct device *dev, -struct device_attribute *attr, -const char *buf,size_t count) -{ -struct pci_dev *pdev =to_pci_dev (dev); -int ret; - - ret =driver_set_override (dev, &pdev->driver_override, buf, count); -if (ret) -return ret; - -return count; -} - -static ssize_t driver_override_show (struct device *dev, -struct device_attribute *attr,char *buf) -{ -struct pci_dev *pdev =to_pci_dev (dev); -ssize_t len; - -device_lock (dev); - len =sysfs_emit (buf,"%s\n" , pdev->driver_override); -device_unlock (dev); -return len; -} -static DEVICE_ATTR_RW (driver_override); -static struct attribute *pci_dev_attrs[] = { &dev_attr_power_state.attr, &dev_attr_resource.attr,
@@ -669,7 +642,6 @@ static struct attribute *pci_dev_attrs[] = {#ifdef CONFIG_OF &dev_attr_devspec.attr, #endif - &dev_attr_driver_override.attr, &dev_attr_ari_enabled.attr, NULL, };
drivers/pci/probe.c +0 -1
@@ -2488,7 +2488,6 @@ static void pci_release_dev(struct device *dev)pci_release_of_node (pci_dev);pcibios_release_device (pci_dev);pci_bus_put (pci_dev->bus); -kfree (pci_dev->driver_override);bitmap_free (pci_dev->dma_alias_mask);dev_dbg (dev,"device released\n" );kfree (pci_dev);
drivers/vfio/pci/vfio_pci_core.c +2 -3
@@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb, pdev->is_virtfn && physfn == vdev->pdev) {pci_info (vdev->pdev,"Captured SR-IOV VF %s driver_override\n" ,pci_name (pdev)); - pdev->driver_override =kasprintf (GFP_KERNEL ,"%s" , - vdev->vdev.ops->name); -WARN_ON (!pdev->driver_override); +WARN_ON (device_set_driver_override (&pdev->dev, + vdev->vdev.ops->name)); } else if (action == BUS_NOTIFY_BOUND_DRIVER && pdev->is_virtfn && physfn == vdev->pdev) {struct pci_driver *drv =pci_dev_driver (pdev);
drivers/xen/xen-pciback/pci_stub.c +4 -2
@@ -598,6 +598,8 @@ static int pcistub_seize(struct pci_dev *dev,return err; } +static struct pci_driver xen_pcibk_pci_driver; +/* Called when 'bind'. This means we must _NOT_ call pci_reset_function or * other functions that take the sysfs lock. */ static int pcistub_probe (struct pci_dev *dev,const struct pci_device_id *id)
@@ -609,8 +611,8 @@ static int pcistub_probe(struct pci_dev *dev, const struct pci_device_id *id) match =pcistub_match (dev); -if ((dev->driver_override && - !strcmp (dev->driver_override,PCISTUB_DRIVER_NAME )) || +if (device_match_driver_override (&dev->dev, + &xen_pcibk_pci_driver.driver) >0 || match) { if (dev->hdr_type != PCI_HEADER_TYPE_NORMAL
include/linux/pci.h +0 -6
@@ -575,12 +575,6 @@ struct pci_dev {u8 supported_speeds;/* Supported Link Speeds Vector */ phys_addr_t rom;/* Physical address if not from BAR */ size_t romlen;/* Length if not from BAR */ -/* -* 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; -unsigned long priv_flags;/* Private flags for the PCI driver */ /* These methods index pci_reset_fn_methods[] */
Signature · 1 line
2.53.0
Re: [PATCH 05/12] PCI: use generic driver_override infrastructure
Quoted 8 lines · Danilo Krummrich
When a driver is probed through
__driver_attach(), the bus'match()callback is called without the device lock held, thus accessing thedriver_overridefield without a lock, which can cause a UAF.Fix this by using the driver-core
driver_overrideinfrastructure taking care of proper locking internally.
Quoted 8 lines
Note that calling
match()from__driver_attach()without the device lock held is intentional. [1]Reported-by Gui-Dong Han <[email protected]>Fixes 782a985d7af2 ("PCI: Introduce new device binding path using pci_dev.driver_override")Signed-off-by Danilo Krummrich <[email protected]>
Tested on QEMU PCI with multiple debug configs enabled. The original PoCs run cleanly without triggering the issue.
Thanks Danilo.
Quoted 179 lines
--- drivers/pci/pci-driver.c | 11 +++++++---- drivers/pci/pci-sysfs.c | 28 ---------------------------- drivers/pci/probe.c | 1 - drivers/vfio/pci/vfio_pci_core.c | 5 ++--- drivers/xen/xen-pciback/pci_stub.c | 6 ++++-- include/linux/pci.h | 6 ------ 6 files changed, 13 insertions(+), 44 deletions(-)drivers/pci/pci-driver.c +7
-4@@ -138,9 +138,11 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv, {struct pci_dynid *dynid;const struct pci_device_id *found_id =NULL , *ids; +int ret;/* When driver_override is set, only bind to the matching driver */ -if (dev->driver_override &&strcmp (dev->driver_override, drv->name)) + ret =device_match_driver_override (&dev->dev, &drv->driver); +if (ret ==0 )return NULL ;/* Look at the dynamic ids first, before the static ones */ @@ -164,7 +166,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,* matching. */ if (found_id->override_only) { -if (dev->driver_override) +if (ret >0 )return found_id; }else {return found_id;@@ -172,7 +174,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv, }/* driver_override will always match, send a dummy id */ -if (dev->driver_override) +if (ret >0 )return &pci_device_id_any;return NULL ; }@@ -452,7 +454,7 @@ static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)static inline bool pci_device_can_probe (struct pci_dev *pdev) {return (!pdev->is_virtfn || pdev->physfn->sriov->drivers_autoprobe || - pdev->driver_override); +device_has_driver_override (&pdev->dev)); }#else static inline bool pci_device_can_probe (struct pci_dev *pdev)@@ -1722,6 +1724,7 @@ static const struct cpumask *pci_device_irq_get_affinity(struct device *dev,const struct bus_type pci_bus_type = { .name ="pci" , + .driver_override = true, .match = pci_bus_match, .uevent = pci_uevent, .probe = pci_device_probe,drivers/pci/pci-sysfs.c +0
-28@@ -615,33 +615,6 @@ static ssize_t devspec_show(struct device *dev,static DEVICE_ATTR_RO (devspec);#endif -static ssize_t driver_override_store (struct device *dev, -struct device_attribute *attr, -const char *buf,size_t count) -{ -struct pci_dev *pdev =to_pci_dev (dev); -int ret; - - ret =driver_set_override (dev, &pdev->driver_override, buf, count); -if (ret) -return ret; - -return count; -} - -static ssize_t driver_override_show (struct device *dev, -struct device_attribute *attr,char *buf) -{ -struct pci_dev *pdev =to_pci_dev (dev); -ssize_t len; - -device_lock (dev); - len =sysfs_emit (buf,"%s\n" , pdev->driver_override); -device_unlock (dev); -return len; -} -static DEVICE_ATTR_RW (driver_override); -static struct attribute *pci_dev_attrs[] = { &dev_attr_power_state.attr, &dev_attr_resource.attr,@@ -669,7 +642,6 @@ static struct attribute *pci_dev_attrs[] = {#ifdef CONFIG_OF &dev_attr_devspec.attr, #endif - &dev_attr_driver_override.attr, &dev_attr_ari_enabled.attr, NULL, };drivers/pci/probe.c +0
-1@@ -2488,7 +2488,6 @@ static void pci_release_dev(struct device *dev)pci_release_of_node (pci_dev);pcibios_release_device (pci_dev);pci_bus_put (pci_dev->bus); -kfree (pci_dev->driver_override);bitmap_free (pci_dev->dma_alias_mask);dev_dbg (dev,"device released\n" );kfree (pci_dev);drivers/vfio/pci/vfio_pci_core.c +2
-3@@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb, pdev->is_virtfn && physfn == vdev->pdev) {pci_info (vdev->pdev,"Captured SR-IOV VF %s driver_override\n" ,pci_name (pdev)); - pdev->driver_override =kasprintf (GFP_KERNEL ,"%s" , - vdev->vdev.ops->name); -WARN_ON (!pdev->driver_override); +WARN_ON (device_set_driver_override (&pdev->dev, + vdev->vdev.ops->name)); } else if (action == BUS_NOTIFY_BOUND_DRIVER && pdev->is_virtfn && physfn == vdev->pdev) {struct pci_driver *drv =pci_dev_driver (pdev);drivers/xen/xen-pciback/pci_stub.c +4
-2@@ -598,6 +598,8 @@ static int pcistub_seize(struct pci_dev *dev,return err; } +static struct pci_driver xen_pcibk_pci_driver; +/* Called when 'bind'. This means we must _NOT_ call pci_reset_function or * other functions that take the sysfs lock. */ static int pcistub_probe (struct pci_dev *dev,const struct pci_device_id *id)@@ -609,8 +611,8 @@ static int pcistub_probe(struct pci_dev *dev, const struct pci_device_id *id) match =pcistub_match (dev); -if ((dev->driver_override && - !strcmp (dev->driver_override,PCISTUB_DRIVER_NAME )) || +if (device_match_driver_override (&dev->dev, + &xen_pcibk_pci_driver.driver) >0 || match) { if (dev->hdr_type != PCI_HEADER_TYPE_NORMALinclude/linux/pci.h +0
-7@@ -575,12 +575,6 @@ struct pci_dev {u8 supported_speeds;/* Supported Link Speeds Vector */ phys_addr_t rom;/* Physical address if not from BAR */ size_t romlen;/* Length if not from BAR */ -/* -* 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; -unsigned long priv_flags;/* Private flags for the PCI driver */ /* These methods index pci_reset_fn_methods[] */ --2.53.0
Re: [PATCH 05/12] PCI: use generic driver_override infrastructure
In reply to Danilo KrummrichQuoted 15 lines · Danilo Krummrich
When a driver is probed through
__driver_attach(), the bus'match()callback is called without the device lock held, thus accessing thedriver_overridefield without a lock, which can cause a UAF.Fix this by using the driver-core
driver_overrideinfrastructure taking care of proper locking internally.Note that calling
match()from__driver_attach()without the device lock held is intentional. [1]Reported-by Gui-Dong Han <[email protected]>Fixes 782a985d7af2 ("PCI: Introduce new device binding path using pci_dev.driver_override")Signed-off-by Danilo Krummrich <[email protected]>
Quoted 5 lines
--- drivers/pci/pci-driver.c | 11 +++++++---- drivers/pci/pci-sysfs.c | 28 ---------------------------- drivers/pci/probe.c | 1 - include/linux/pci.h | 6 ------
For the above:
"driver_override" is mentioned several places in Documentation/ABI/testing/sysfs-bus-*. I assume this series doesn't change the behavior documented there? Should any of this doc be consolidated?
Quoted 174 lines
drivers/vfio/pci/vfio_pci_core.c | 5 ++--- drivers/xen/xen-pciback/pci_stub.c | 6 ++++-- 6 files changed, 13 insertions(+), 44 deletions(-)drivers/pci/pci-driver.c +7
-4@@ -138,9 +138,11 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv, {struct pci_dynid *dynid;const struct pci_device_id *found_id =NULL , *ids; +int ret;/* When driver_override is set, only bind to the matching driver */ -if (dev->driver_override &&strcmp (dev->driver_override, drv->name)) + ret =device_match_driver_override (&dev->dev, &drv->driver); +if (ret ==0 )return NULL ;/* Look at the dynamic ids first, before the static ones */ @@ -164,7 +166,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,* matching. */ if (found_id->override_only) { -if (dev->driver_override) +if (ret >0 )return found_id; }else {return found_id;@@ -172,7 +174,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv, }/* driver_override will always match, send a dummy id */ -if (dev->driver_override) +if (ret >0 )return &pci_device_id_any;return NULL ; }@@ -452,7 +454,7 @@ static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)static inline bool pci_device_can_probe (struct pci_dev *pdev) {return (!pdev->is_virtfn || pdev->physfn->sriov->drivers_autoprobe || - pdev->driver_override); +device_has_driver_override (&pdev->dev)); }#else static inline bool pci_device_can_probe (struct pci_dev *pdev)@@ -1722,6 +1724,7 @@ static const struct cpumask *pci_device_irq_get_affinity(struct device *dev,const struct bus_type pci_bus_type = { .name ="pci" , + .driver_override = true, .match = pci_bus_match, .uevent = pci_uevent, .probe = pci_device_probe,drivers/pci/pci-sysfs.c +0
-28@@ -615,33 +615,6 @@ static ssize_t devspec_show(struct device *dev,static DEVICE_ATTR_RO (devspec);#endif -static ssize_t driver_override_store (struct device *dev, -struct device_attribute *attr, -const char *buf,size_t count) -{ -struct pci_dev *pdev =to_pci_dev (dev); -int ret; - - ret =driver_set_override (dev, &pdev->driver_override, buf, count); -if (ret) -return ret; - -return count; -} - -static ssize_t driver_override_show (struct device *dev, -struct device_attribute *attr,char *buf) -{ -struct pci_dev *pdev =to_pci_dev (dev); -ssize_t len; - -device_lock (dev); - len =sysfs_emit (buf,"%s\n" , pdev->driver_override); -device_unlock (dev); -return len; -} -static DEVICE_ATTR_RW (driver_override); -static struct attribute *pci_dev_attrs[] = { &dev_attr_power_state.attr, &dev_attr_resource.attr,@@ -669,7 +642,6 @@ static struct attribute *pci_dev_attrs[] = {#ifdef CONFIG_OF &dev_attr_devspec.attr, #endif - &dev_attr_driver_override.attr, &dev_attr_ari_enabled.attr, NULL, };drivers/pci/probe.c +0
-1@@ -2488,7 +2488,6 @@ static void pci_release_dev(struct device *dev)pci_release_of_node (pci_dev);pcibios_release_device (pci_dev);pci_bus_put (pci_dev->bus); -kfree (pci_dev->driver_override);bitmap_free (pci_dev->dma_alias_mask);dev_dbg (dev,"device released\n" );kfree (pci_dev);drivers/vfio/pci/vfio_pci_core.c +2
-3@@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb, pdev->is_virtfn && physfn == vdev->pdev) {pci_info (vdev->pdev,"Captured SR-IOV VF %s driver_override\n" ,pci_name (pdev)); - pdev->driver_override =kasprintf (GFP_KERNEL ,"%s" , - vdev->vdev.ops->name); -WARN_ON (!pdev->driver_override); +WARN_ON (device_set_driver_override (&pdev->dev, + vdev->vdev.ops->name)); } else if (action == BUS_NOTIFY_BOUND_DRIVER && pdev->is_virtfn && physfn == vdev->pdev) {struct pci_driver *drv =pci_dev_driver (pdev);drivers/xen/xen-pciback/pci_stub.c +4
-2@@ -598,6 +598,8 @@ static int pcistub_seize(struct pci_dev *dev,return err; } +static struct pci_driver xen_pcibk_pci_driver; +/* Called when 'bind'. This means we must _NOT_ call pci_reset_function or * other functions that take the sysfs lock. */ static int pcistub_probe (struct pci_dev *dev,const struct pci_device_id *id)@@ -609,8 +611,8 @@ static int pcistub_probe(struct pci_dev *dev, const struct pci_device_id *id) match =pcistub_match (dev); -if ((dev->driver_override && - !strcmp (dev->driver_override,PCISTUB_DRIVER_NAME )) || +if (device_match_driver_override (&dev->dev, + &xen_pcibk_pci_driver.driver) >0 || match) { if (dev->hdr_type != PCI_HEADER_TYPE_NORMALinclude/linux/pci.h +0
-6@@ -575,12 +575,6 @@ struct pci_dev {u8 supported_speeds;/* Supported Link Speeds Vector */ phys_addr_t rom;/* Physical address if not from BAR */ size_t romlen;/* Length if not from BAR */ -/* -* 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; -unsigned long priv_flags;/* Private flags for the PCI driver */ /* These methods index pci_reset_fn_methods[] */ Signature · 1 line
2.53.0
Danilo Krummrich
maintainerRe: [PATCH 05/12] PCI: use generic driver_override infrastructure
Quoted 22 lines · Bjorn Helgaas
Quoted 15 lines · Danilo Krummrich
When a driver is probed through
__driver_attach(), the bus'match()callback is called without the device lock held, thus accessing thedriver_overridefield without a lock, which can cause a UAF.Fix this by using the driver-core
driver_overrideinfrastructure taking care of proper locking internally.Note that calling
match()from__driver_attach()without the device lock held is intentional. [1]Reported-by Gui-Dong Han <[email protected]>Fixes 782a985d7af2 ("PCI: Introduce new device binding path using pci_dev.driver_override")Signed-off-by Danilo Krummrich <[email protected]>Quoted 5 lines
--- drivers/pci/pci-driver.c | 11 +++++++---- drivers/pci/pci-sysfs.c | 28 ---------------------------- drivers/pci/probe.c | 1 - include/linux/pci.h | 6 ------
Quoted 7 lines
For the above:
Acked-by Bjorn Helgaas <[email protected]>"
driver_override" is mentioned several places inDocumentation/ABI/testing/sysfs-bus-*. I assume this series doesn't change the behavior documented there?
Correct, none of this is altered.
Danilo Krummrich
maintainerRe: [PATCH 05/12] PCI: use generic driver_override infrastructure
In reply to Danilo Krummrich(Cc: Jason)
drivers/vfio/pci/vfio_pci_core.c static int vfio_pci_bus_notifier(struct notifier_block *nb, · line 1991
Quoted 13 lines · Danilo Krummrich
drivers/vfio/pci/vfio_pci_core.c +2
-3@@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb, pdev->is_virtfn && physfn == vdev->pdev) {pci_info (vdev->pdev,"Captured SR-IOV VF %s driver_override\n" ,pci_name (pdev)); - pdev->driver_override =kasprintf (GFP_KERNEL ,"%s" , - vdev->vdev.ops->name); -WARN_ON (!pdev->driver_override); +WARN_ON (device_set_driver_override (&pdev->dev, + vdev->vdev.ops->name));
Technically, this is a change in behavior. If vdev->vdev.ops->name is NULL, it will trigger the WARN_ON(), whereas before it would have just written "(null)" into driver_override.
I assume that vfio_pci_core drivers are expected to set the name in struct vfio_device_ops in the first place and this code (silently) relies on this invariant?
Alex, Jason: Should we keep this hunk above as is and check for a proper name in struct vfio_device_ops in vfio_pci_core_register_device() with a subsequent patch?
Quoted 3 lines
drivers/vfio/pci/vfio_pci_core.c +0
-0}else if (action ==BUS_NOTIFY_BOUND_DRIVER && pdev->is_virtfn && physfn == vdev->pdev) {struct pci_driver *drv =pci_dev_driver (pdev);
Alex Williamson
maintainerRe: [PATCH 05/12] PCI: use generic driver_override infrastructure
Quoted 12 lines · On Mon, 30 Mar 2026 19:38:41 +0200 "Danilo Krummrich" <[email protected]> wrote:
(Cc: Jason)
Quoted 9 lines · Danilo Krummrich
drivers/vfio/pci/vfio_pci_core.c +0
-1@@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb, pdev->is_virtfn && physfn == vdev->pdev) {pci_info (vdev->pdev,"Captured SR-IOV VF %s driver_override\n" ,pci_name (pdev)); - pdev->driver_override = kasprintf(GFP_KERNEL, "%s",
Quoted 8 lines
drivers/vfio/pci/vfio_pci_core.c static int vfio_pci_bus_notifier(struct notifier_block *nb, · line 1991
Quoted 4 lines
drivers/vfio/pci/vfio_pci_core.c +2
-2- vdev->vdev.ops->name); -WARN_ON (!pdev->driver_override); +WARN_ON (device_set_driver_override (&pdev->dev, + vdev->vdev.ops->name));Technically, this is a change in behavior. If
vdev->vdev.ops->nameis NULL, it will trigger theWARN_ON(), whereas before it would have just written "(null)" intodriver_override.
It's worse than that. Looking at the implementation in [1], we have:
+static inline int device_set_driver_override(struct device *dev, const char *s) +{
+ return __device_set_driver_override(dev, s, strlen(s));
+}
So if name is NULL, we oops in strlen() before we even hit the -EINVAL and WARN_ON().
I don't believe we have any vfio-pci variant drivers where the name is NULL, but kasprintf() handling NULL as "(null)" was a consideration in this design, that even if there is no name the device is sequestered with a driver_override that won't match an actual driver.
Quoted 3 lines
I assume that
vfio_pci_coredrivers are expected to set the name in structvfio_device_opsin the first place and this code (silently) relies on this invariant?
We do expect that, but it was previously safe either way to make sure VFs are only bound to the same ops driver or barring that, at least don't perform a standard driver match. The last thing we want to happen automatically is for a user owned PF to create SR-IOV VFs that automatically bind to native kernel drivers.
Quoted 3 lines
Alex, Jason: Should we keep this hunk above as is and check for a proper name in struct
vfio_device_opsinvfio_pci_core_register_device()with a subsequent patch?
Given the oops, my preference would be to roll it in here. This change is what makes it a requirement that name cannot be NULL, where this was safely handled with kasprintf(). Thanks,
Alex
[1] https://lore.kernel.org/all/[email protected]/
Quoted 4 lines
Quoted 4 lines
drivers/vfio/pci/vfio_pci_core.c +0
-0}else if (action ==BUS_NOTIFY_BOUND_DRIVER && pdev->is_virtfn && physfn == vdev->pdev) {struct pci_driver *drv =pci_dev_driver (pdev);
Danilo Krummrich
maintainerRe: [PATCH 05/12] PCI: use generic driver_override infrastructure
Quoted 31 lines · Alex Williamson
Quoted 12 lines · On Mon, 30 Mar 2026 19:38:41 +0200 "Danilo Krummrich" <[email protected]> wrote:
(Cc: Jason)
Quoted 9 lines · Danilo Krummrich
drivers/vfio/pci/vfio_pci_core.c +0
-1@@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb, pdev->is_virtfn && physfn == vdev->pdev) {pci_info (vdev->pdev,"Captured SR-IOV VF %s driver_override\n" ,pci_name (pdev)); - pdev->driver_override = kasprintf(GFP_KERNEL, "%s",Quoted 8 lines
drivers/vfio/pci/vfio_pci_core.c static int vfio_pci_bus_notifier(struct notifier_block *nb, · line 1991
Quoted 4 lines
drivers/vfio/pci/vfio_pci_core.c +2
-2- vdev->vdev.ops->name); -WARN_ON (!pdev->driver_override); +WARN_ON (device_set_driver_override (&pdev->dev, + vdev->vdev.ops->name));Technically, this is a change in behavior. If
vdev->vdev.ops->nameis NULL, it will trigger theWARN_ON(), whereas before it would have just written "(null)" intodriver_override.It's worse than that. Looking at the implementation in [1], we have:
(quoted diff) +4
-0+static inline int device_set_driver_override(struct device *dev, const char *s) +{ + return __device_set_driver_override(dev, s, strlen(s)); +}
Quoted 2 lines
So if name is NULL, we oops in
strlen()before we even hit the-EINVALandWARN_ON().
This was changed in v2 [2] and the actual code in-tree is
static inline int device_set_driver_override(struct device *dev, const char *s)
{
return __device_set_driver_override(dev, s, s ? strlen(s) : 0);
}so it does indeed return -EINVAL for a NULL pointer.
Quoted 15 lines
I don't believe we have any vfio-pci variant drivers where the name is NULL, but
kasprintf()handling NULL as "(null)" was a consideration in this design, that even if there is no name the device is sequestered with adriver_overridethat won't match an actual driver.Quoted 3 lines
I assume that
vfio_pci_coredrivers are expected to set the name in structvfio_device_opsin the first place and this code (silently) relies on this invariant?We do expect that, but it was previously safe either way to make sure VFs are only bound to the same ops driver or barring that, at least don't perform a standard driver match. The last thing we want to happen automatically is for a user owned PF to create SR-IOV VFs that automatically bind to native kernel drivers.
Quoted 7 lines
Quoted 3 lines
Alex, Jason: Should we keep this hunk above as is and check for a proper name in struct
vfio_device_opsinvfio_pci_core_register_device()with a subsequent patch?Given the oops, my preference would be to roll it in here. This change is what makes it a requirement that name cannot be NULL, where this was safely handled with
kasprintf().
Again, no oops here. :)
I still think it makes more sense to fail early in vfio_pci_core_register_device(), rather than silently accept "(null)" in driver_override. It also doesn't seem unreasonable with only the WARN_ON(), but I can also just add vdev->vdev.ops->name ?: "(null)".
Please let me know what you prefer.
- Danilo
Quoted 1 line
Danilo Krummrich
maintainerRe: [PATCH 05/12] PCI: use generic driver_override infrastructure
Quoted 67 lines · Danilo Krummrich
Quoted 31 lines · Alex Williamson
Quoted 12 lines · On Mon, 30 Mar 2026 19:38:41 +0200 "Danilo Krummrich" <[email protected]> wrote:
(Cc: Jason)
Quoted 9 lines · Danilo Krummrich
drivers/vfio/pci/vfio_pci_core.c +0
-1@@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb, pdev->is_virtfn && physfn == vdev->pdev) {pci_info (vdev->pdev,"Captured SR-IOV VF %s driver_override\n" ,pci_name (pdev)); - pdev->driver_override = kasprintf(GFP_KERNEL, "%s",Quoted 8 lines
drivers/vfio/pci/vfio_pci_core.c static int vfio_pci_bus_notifier(struct notifier_block *nb, · line 1991
Quoted 4 lines
drivers/vfio/pci/vfio_pci_core.c +2
-2- vdev->vdev.ops->name); -WARN_ON (!pdev->driver_override); +WARN_ON (device_set_driver_override (&pdev->dev, + vdev->vdev.ops->name));Technically, this is a change in behavior. If
vdev->vdev.ops->nameis NULL, it will trigger theWARN_ON(), whereas before it would have just written "(null)" intodriver_override.It's worse than that. Looking at the implementation in [1], we have:
(quoted diff) +4
-0+static inline int device_set_driver_override(struct device *dev, const char *s) +{ + return __device_set_driver_override(dev, s, strlen(s)); +}Quoted 2 lines
So if name is NULL, we oops in
strlen()before we even hit the-EINVALandWARN_ON().This was changed in v2 [2] and the actual code in-tree is
static inline int device_set_driver_override(struct device *dev, const char *s) { return __device_set_driver_override(dev, s, s ? strlen(s) : 0); }so it does indeed return
-EINVALfor a NULL pointer.Quoted 15 lines
I don't believe we have any vfio-pci variant drivers where the name is NULL, but
kasprintf()handling NULL as "(null)" was a consideration in this design, that even if there is no name the device is sequestered with adriver_overridethat won't match an actual driver.Quoted 3 lines
I assume that
vfio_pci_coredrivers are expected to set the name in structvfio_device_opsin the first place and this code (silently) relies on this invariant?We do expect that, but it was previously safe either way to make sure VFs are only bound to the same ops driver or barring that, at least don't perform a standard driver match. The last thing we want to happen automatically is for a user owned PF to create SR-IOV VFs that automatically bind to native kernel drivers.
Quoted 7 lines
Quoted 3 lines
Alex, Jason: Should we keep this hunk above as is and check for a proper name in struct
vfio_device_opsinvfio_pci_core_register_device()with a subsequent patch?Given the oops, my preference would be to roll it in here. This change is what makes it a requirement that name cannot be NULL, where this was safely handled with
kasprintf().
Quoted 6 lines
Again, no oops here. :)
I still think it makes more sense to fail early in
vfio_pci_core_register_device(), rather than silently accept "(null)" indriver_override. It also doesn't seem unreasonable with only theWARN_ON(), but I can also just addvdev->vdev.ops->name?: "(null)".
(Or just skip the call if !vdev->vdev.ops->name, as a user will read "(null)" from sysfs either way.)
Quoted 7 lines
Please let me know what you prefer.
- Danilo
Quoted 1 line
Re: [PATCH 05/12] PCI: use generic driver_override infrastructure
Quoted 38 lines · On Tue, 31 Mar 2026 10:22:14 +0200 "Danilo Krummrich" <[email protected]> wrote:
Quoted 37 lines · Danilo Krummrich
Quoted 31 lines · Alex Williamson
Quoted 12 lines · On Mon, 30 Mar 2026 19:38:41 +0200 "Danilo Krummrich" <[email protected]> wrote:
(Cc: Jason)
Quoted 9 lines · Danilo Krummrich
drivers/vfio/pci/vfio_pci_core.c +0
-1@@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb, pdev->is_virtfn && physfn == vdev->pdev) {pci_info (vdev->pdev,"Captured SR-IOV VF %s driver_override\n" ,pci_name (pdev)); - pdev->driver_override = kasprintf(GFP_KERNEL, "%s",Quoted 8 lines
drivers/vfio/pci/vfio_pci_core.c static int vfio_pci_bus_notifier(struct notifier_block *nb, · line 1991
Quoted 4 lines
drivers/vfio/pci/vfio_pci_core.c +2
-2- vdev->vdev.ops->name); -WARN_ON (!pdev->driver_override); +WARN_ON (device_set_driver_override (&pdev->dev, + vdev->vdev.ops->name));Technically, this is a change in behavior. If
vdev->vdev.ops->nameis NULL, it will trigger theWARN_ON(), whereas before it would have just written "(null)" intodriver_override.It's worse than that. Looking at the implementation in [1], we have:
(quoted diff) +4
-0+static inline int device_set_driver_override(struct device *dev, const char *s) +{ + return __device_set_driver_override(dev, s, strlen(s)); +}Quoted 2 lines
So if name is NULL, we oops in
strlen()before we even hit the-EINVALandWARN_ON().This was changed in v2 [2] and the actual code in-tree is
Quoted 6 lines
Quoted 6 lines
static inline int device_set_driver_override(struct device *dev, const char *s) { return __device_set_driver_override(dev, s, s ? strlen(s) : 0); }so it does indeed return
-EINVALfor a NULL pointer.
Ok, good.
Quoted 25 lines
Quoted 25 lines
Quoted 15 lines
I don't believe we have any vfio-pci variant drivers where the name is NULL, but
kasprintf()handling NULL as "(null)" was a consideration in this design, that even if there is no name the device is sequestered with adriver_overridethat won't match an actual driver.Quoted 3 lines
I assume that
vfio_pci_coredrivers are expected to set the name in structvfio_device_opsin the first place and this code (silently) relies on this invariant?We do expect that, but it was previously safe either way to make sure VFs are only bound to the same ops driver or barring that, at least don't perform a standard driver match. The last thing we want to happen automatically is for a user owned PF to create SR-IOV VFs that automatically bind to native kernel drivers.
Quoted 7 lines
Quoted 3 lines
Alex, Jason: Should we keep this hunk above as is and check for a proper name in struct
vfio_device_opsinvfio_pci_core_register_device()with a subsequent patch?Given the oops, my preference would be to roll it in here. This change is what makes it a requirement that name cannot be NULL, where this was safely handled with
kasprintf().Again, no oops here. :)
Quoted 7 lines
Quoted 4 lines
I still think it makes more sense to fail early in
vfio_pci_core_register_device(), rather than silently accept "(null)" indriver_override. It also doesn't seem unreasonable with only theWARN_ON(), but I can also just addvdev->vdev.ops->name?: "(null)".(Or just skip the call if !vdev->vdev.ops->name, as a user will read "(null)" from sysfs either way.)
Hmm, I suppose they would, but there's a fundamental difference between driver_override being set to "(null)" vs being NULL and only interpreted as "(null)" via show. The former would prevent ID table matching, the latter would not. The former is what was intended here, without realizing both look the same through sysfs and present a difficult debugging challenge.
Given no oops for strlen() now and no known in-kernel drivers with a NULL name, I can post a patch that rejects a NULL name in the ops structure in vfio_pci_core_register_device(), avoiding the entire situation.
For this,
Thanks,
Alex
Danilo Krummrich
maintainerPATCH 6/12 platform/wmi: use generic driver_override infrastructure
In reply to Danilo KrummrichWhen 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/platform/wmi/core.c | 36 +++++------------------------------- include/linux/wmi.h | 4 ---- 2 files changed, 5 insertions(+), 35 deletions(-)
drivers/platform/wmi/core.c +5 -31
@@ -842,39 +842,11 @@ static ssize_t expensive_show(struct device *dev, }static DEVICE_ATTR_RO (expensive); -static ssize_t driver_override_show (struct device *dev,struct device_attribute *attr, -char *buf) -{ -struct wmi_device *wdev =to_wmi_device (dev); -ssize_t ret; - -device_lock (dev); - ret =sysfs_emit (buf,"%s\n" , wdev->driver_override); -device_unlock (dev); - -return ret; -} - -static ssize_t driver_override_store (struct device *dev,struct device_attribute *attr, -const char *buf,size_t count) -{ -struct wmi_device *wdev =to_wmi_device (dev); -int ret; - - ret =driver_set_override (dev, &wdev->driver_override, buf, count); -if (ret <0 ) -return ret; - -return count; -} -static DEVICE_ATTR_RW (driver_override); -static struct attribute *wmi_attrs[] = { &dev_attr_modalias.attr, &dev_attr_guid.attr, &dev_attr_instance_count.attr, &dev_attr_expensive.attr, - &dev_attr_driver_override.attr,NULL };ATTRIBUTE_GROUPS (wmi);
@@ -943,7 +915,6 @@ static void wmi_dev_release(struct device *dev) {struct wmi_block *wblock =dev_to_wblock (dev); -kfree (wblock->dev.driver_override);kfree (wblock); }
@@ -952,10 +923,12 @@ static int wmi_dev_match(struct device *dev, const struct device_driver *driver)const struct wmi_driver *wmi_driver =to_wmi_driver (driver);struct wmi_block *wblock =dev_to_wblock (dev);const struct wmi_device_id *id = wmi_driver->id_table; +int ret;/* When driver_override is set, only bind to the matching driver */ -if (wblock->dev.driver_override) -return !strcmp (wblock->dev.driver_override, driver->name); + ret =device_match_driver_override (dev, driver); +if (ret >=0 ) +return ret;if (id ==NULL )return 0 ;
@@ -1076,6 +1049,7 @@ static struct class wmi_bus_class = {static const struct bus_type wmi_bus_type = { .name ="wmi" , .dev_groups = wmi_groups, + .driver_override = true, .match = wmi_dev_match, .uevent = wmi_dev_uevent, .probe = wmi_dev_probe,
include/linux/wmi.h +0 -4
@@ -18,16 +18,12 @@* struct wmi_device - WMI device structure * @dev: Device associated with this WMI device * @setable: True for devices implementing the Set Control Method -* @driver_override: Driver name to force a match; do not set directly, -* because core frees it; use driver_set_override() to -* set or clear it. * * This represents WMI devices discovered by the WMI driver core. */ struct wmi_device {struct device dev;bool setable; -const char *driver_override; };/**
Signature · 1 line
2.53.0
Re: [PATCH 06/12] platform/wmi: use generic driver_override infrastructure
Am 24.03.26 um 01:59 schrieb Danilo Krummrich:
Quoted 9 lines
When a driver is probed through
__driver_attach(), the bus'match()callback is called without the device lock held, thus accessing thedriver_overridefield without a lock, which can cause a UAF.Fix this by using the driver-core
driver_overrideinfrastructure taking care of proper locking internally.Note that calling
match()from__driver_attach()without the device lock held is intentional. [1]
Quoted 106 lines
Reported-by Gui-Dong Han <[email protected]>Fixes 12046f8c77e0 ("platform/x86: wmi: Add driver_override support")Signed-off-by Danilo Krummrich <[email protected]>--- drivers/platform/wmi/core.c | 36 +++++------------------------------- include/linux/wmi.h | 4 ---- 2 files changed, 5 insertions(+), 35 deletions(-)drivers/platform/wmi/core.c +5
-31@@ -842,39 +842,11 @@ static ssize_t expensive_show(struct device *dev, }static DEVICE_ATTR_RO (expensive); -static ssize_t driver_override_show (struct device *dev,struct device_attribute *attr, -char *buf) -{ -struct wmi_device *wdev =to_wmi_device (dev); -ssize_t ret; - -device_lock (dev); - ret =sysfs_emit (buf,"%s\n" , wdev->driver_override); -device_unlock (dev); - -return ret; -} - -static ssize_t driver_override_store (struct device *dev,struct device_attribute *attr, -const char *buf,size_t count) -{ -struct wmi_device *wdev =to_wmi_device (dev); -int ret; - - ret =driver_set_override (dev, &wdev->driver_override, buf, count); -if (ret <0 ) -return ret; - -return count; -} -static DEVICE_ATTR_RW (driver_override); -static struct attribute *wmi_attrs[] = { &dev_attr_modalias.attr, &dev_attr_guid.attr, &dev_attr_instance_count.attr, &dev_attr_expensive.attr, - &dev_attr_driver_override.attr,NULL };ATTRIBUTE_GROUPS (wmi);@@ -943,7 +915,6 @@ static void wmi_dev_release(struct device *dev) {struct wmi_block *wblock =dev_to_wblock (dev); -kfree (wblock->dev.driver_override);kfree (wblock); }@@ -952,10 +923,12 @@ static int wmi_dev_match(struct device *dev, const struct device_driver *driver)const struct wmi_driver *wmi_driver =to_wmi_driver (driver);struct wmi_block *wblock =dev_to_wblock (dev);const struct wmi_device_id *id = wmi_driver->id_table; +int ret;/* When driver_override is set, only bind to the matching driver */ -if (wblock->dev.driver_override) -return !strcmp (wblock->dev.driver_override, driver->name); + ret =device_match_driver_override (dev, driver); +if (ret >=0 ) +return ret;if (id ==NULL )return 0 ;@@ -1076,6 +1049,7 @@ static struct class wmi_bus_class = {static const struct bus_type wmi_bus_type = { .name ="wmi" , .dev_groups = wmi_groups, + .driver_override = true, .match = wmi_dev_match, .uevent = wmi_dev_uevent, .probe = wmi_dev_probe,include/linux/wmi.h +0
-4@@ -18,16 +18,12 @@* struct wmi_device - WMI device structure * @dev: Device associated with this WMI device * @setable: True for devices implementing the Set Control Method -* @driver_override: Driver name to force a match; do not set directly, -* because core frees it; use driver_set_override() to -* set or clear it. * * This represents WMI devices discovered by the WMI driver core. */ struct wmi_device {struct device dev;bool setable; -const char *driver_override; };/**
Ilpo Järvinen
maintainerRe: [PATCH 06/12] platform/wmi: use generic driver_override infrastructure
In reply to Danilo KrummrichQuoted 107 lines · Danilo Krummrich
When a driver is probed through
__driver_attach(), the bus'match()callback is called without the device lock held, thus accessing thedriver_overridefield without a lock, which can cause a UAF.Fix this by using the driver-core
driver_overrideinfrastructure taking care of proper locking internally.Note that calling
match()from__driver_attach()without the device lock held is intentional. [1]Reported-by Gui-Dong Han <[email protected]>Fixes 12046f8c77e0 ("platform/x86: wmi: Add driver_override support")Signed-off-by Danilo Krummrich <[email protected]>--- drivers/platform/wmi/core.c | 36 +++++------------------------------- include/linux/wmi.h | 4 ---- 2 files changed, 5 insertions(+), 35 deletions(-)drivers/platform/wmi/core.c +5
-31@@ -842,39 +842,11 @@ static ssize_t expensive_show(struct device *dev, }static DEVICE_ATTR_RO (expensive); -static ssize_t driver_override_show (struct device *dev,struct device_attribute *attr, -char *buf) -{ -struct wmi_device *wdev =to_wmi_device (dev); -ssize_t ret; - -device_lock (dev); - ret =sysfs_emit (buf,"%s\n" , wdev->driver_override); -device_unlock (dev); - -return ret; -} - -static ssize_t driver_override_store (struct device *dev,struct device_attribute *attr, -const char *buf,size_t count) -{ -struct wmi_device *wdev =to_wmi_device (dev); -int ret; - - ret =driver_set_override (dev, &wdev->driver_override, buf, count); -if (ret <0 ) -return ret; - -return count; -} -static DEVICE_ATTR_RW (driver_override); -static struct attribute *wmi_attrs[] = { &dev_attr_modalias.attr, &dev_attr_guid.attr, &dev_attr_instance_count.attr, &dev_attr_expensive.attr, - &dev_attr_driver_override.attr,NULL };ATTRIBUTE_GROUPS (wmi);@@ -943,7 +915,6 @@ static void wmi_dev_release(struct device *dev) {struct wmi_block *wblock =dev_to_wblock (dev); -kfree (wblock->dev.driver_override);kfree (wblock); }@@ -952,10 +923,12 @@ static int wmi_dev_match(struct device *dev, const struct device_driver *driver)const struct wmi_driver *wmi_driver =to_wmi_driver (driver);struct wmi_block *wblock =dev_to_wblock (dev);const struct wmi_device_id *id = wmi_driver->id_table; +int ret;/* When driver_override is set, only bind to the matching driver */ -if (wblock->dev.driver_override) -return !strcmp (wblock->dev.driver_override, driver->name); + ret =device_match_driver_override (dev, driver); +if (ret >=0 ) +return ret;if (id ==NULL )return 0 ;@@ -1076,6 +1049,7 @@ static struct class wmi_bus_class = {static const struct bus_type wmi_bus_type = { .name ="wmi" , .dev_groups = wmi_groups, + .driver_override = true, .match = wmi_dev_match, .uevent = wmi_dev_uevent, .probe = wmi_dev_probe,include/linux/wmi.h +0
-3@@ -18,16 +18,12 @@* struct wmi_device - WMI device structure * @dev: Device associated with this WMI device * @setable: True for devices implementing the Set Control Method -* @driver_override: Driver name to force a match; do not set directly, -* because core frees it; use driver_set_override() to -* set or clear it.
include/linux/wmi.h line 29
Quoted 10 lines
include/linux/wmi.h +0
-1* * This represents WMI devices discovered by the WMI driver core. */ struct wmi_device {struct device dev;bool setable; -const char *driver_override; };/**
Hi,
I tried applying this to platform-drivers tree but it failed to compile so I ended up dropping the changed.
Signature · 1 line
i.
Danilo Krummrich
maintainerRe: [PATCH 06/12] platform/wmi: use generic driver_override infrastructure
Quoted 2 lines · Ilpo Järvinen
I tried applying this to platform-drivers tree but it failed to compile so I ended up dropping the changed.
As the cover letter mentions, it sits on top of v7.0-rc5, did you consider this?
I can also pick it up via the driver-core tree.
Thanks,
Danilo
Re: [PATCH 06/12] platform/wmi: use generic driver_override infrastructure
Quoted 5 lines · Danilo Krummrich
Quoted 2 lines · Ilpo Järvinen
I tried applying this to platform-drivers tree but it failed to compile so I ended up dropping the changed.
As the cover letter mentions, it sits on top of v7.0-rc5, did you consider this?
I noticed it but I just assumed you were working on top of that, not that there's something past -rc1 that is required.
Quoted 1 line
I can also pick it up via the driver-core tree.
If there's some post -rc1 material this depends, it's probably better that way.
Signature · 1 line
i.
Danilo Krummrich
maintainerPATCH 7/12 rpmsg: use generic driver_override infrastructure
In reply to Danilo KrummrichWhen 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/rpmsg/qcom_glink_native.c | 2 -- drivers/rpmsg/rpmsg_core.c | 43 +++++-------------------------- drivers/rpmsg/virtio_rpmsg_bus.c | 1 - include/linux/rpmsg.h | 4 --- 4 files changed, 7 insertions(+), 43 deletions(-)
drivers/rpmsg/qcom_glink_native.c +0 -2
@@ -1623,7 +1623,6 @@ static void qcom_glink_rpdev_release(struct device *dev) {struct rpmsg_device *rpdev =to_rpmsg_device (dev); -kfree (rpdev->driver_override);kfree (rpdev); }
@@ -1859,7 +1858,6 @@ static void qcom_glink_device_release(struct device *dev)/* Release qcom_glink_alloc_channel() reference */ kref_put (&channel->refcount, qcom_glink_channel_release); -kfree (rpdev->driver_override);kfree (rpdev); }
drivers/rpmsg/rpmsg_core.c +7 -36
81 lines not on this page · raw message
drivers/rpmsg/virtio_rpmsg_bus.c +0 -1
@@ -373,7 +373,6 @@ static void virtio_rpmsg_release_device(struct device *dev)struct rpmsg_device *rpdev =to_rpmsg_device (dev);struct virtio_rpmsg_channel *vch =to_virtio_rpmsg_channel (rpdev); -kfree (rpdev->driver_override);kfree (vch); }
include/linux/rpmsg.h +0 -4
@@ -41,9 +41,6 @@ struct rpmsg_channel_info {* rpmsg_device - device that belong to the rpmsg bus * @dev: the device struct * @id: device id (used to match between rpmsg drivers and devices) -* @driver_override: driver name to force a match; do not set directly, -* because core frees it; use driver_set_override() to -* set or clear it. * @src: local address * @dst: destination address * @ept: the rpmsg endpoint of this channel
@@ -53,7 +50,6 @@ struct rpmsg_channel_info {struct rpmsg_device {struct device dev;struct rpmsg_device_id id; -const char *driver_override;u32 src;u32 dst;struct rpmsg_endpoint *ept;
Signature · 1 line
2.53.0
Re: [PATCH 07/12] rpmsg: use generic driver_override infrastructure
Quoted 11 lines · Danilo Krummrich
When a driver is probed through
__driver_attach(), the bus'match()callback is called without the device lock held, thus accessing thedriver_overridefield without a lock, which can cause a UAF.Fix this by using the driver-core
driver_overrideinfrastructure taking care of proper locking internally.Note that calling
match()from__driver_attach()without the device lock held is intentional. [1]
Quoted 7 lines
Reported-by Gui-Dong Han <[email protected]>Fixes e95060478244 ("rpmsg: Introduce a driver override mechanism")Signed-off-by Danilo Krummrich <[email protected]>--- drivers/rpmsg/qcom_glink_native.c | 2 --
For the below files:
Quoted 153 lines
drivers/rpmsg/rpmsg_core.c | 43 +++++-------------------------- drivers/rpmsg/virtio_rpmsg_bus.c | 1 - include/linux/rpmsg.h | 4 --- 4 files changed, 7 insertions(+), 43 deletions(-)drivers/rpmsg/qcom_glink_native.c +0
-2@@ -1623,7 +1623,6 @@ static void qcom_glink_rpdev_release(struct device *dev) {struct rpmsg_device *rpdev =to_rpmsg_device (dev); -kfree (rpdev->driver_override);kfree (rpdev); }@@ -1859,7 +1858,6 @@ static void qcom_glink_device_release(struct device *dev)/* Release qcom_glink_alloc_channel() reference */ kref_put (&channel->refcount, qcom_glink_channel_release); -kfree (rpdev->driver_override);kfree (rpdev); }drivers/rpmsg/rpmsg_core.c +7
-36@@ -358,33 +358,6 @@ rpmsg_show_attr(src, src, "0x%x\n");rpmsg_show_attr (dst, dst,"0x%x\n" );rpmsg_show_attr (announce, announce ?"true" :"false" ,"%s\n" ); -static ssize_t driver_override_store (struct device *dev, -struct device_attribute *attr, -const char *buf,size_t count) -{ -struct rpmsg_device *rpdev =to_rpmsg_device (dev); -int ret; - - ret =driver_set_override (dev, &rpdev->driver_override, buf, count); -if (ret) -return ret; - -return count; -} - -static ssize_t driver_override_show (struct device *dev, -struct device_attribute *attr,char *buf) -{ -struct rpmsg_device *rpdev =to_rpmsg_device (dev); -ssize_t len; - -device_lock (dev); - len =sysfs_emit (buf,"%s\n" , rpdev->driver_override); -device_unlock (dev); -return len; -} -static DEVICE_ATTR_RW (driver_override); -static ssize_t modalias_show (struct device *dev,struct device_attribute *attr,char *buf) {@@ -405,7 +378,6 @@ static struct attribute *rpmsg_dev_attrs[] = { &dev_attr_dst.attr, &dev_attr_src.attr, &dev_attr_announce.attr, - &dev_attr_driver_override.attr,NULL , };ATTRIBUTE_GROUPS (rpmsg_dev);@@ -424,9 +396,11 @@ static int rpmsg_dev_match(struct device *dev, const struct device_driver *drv)const struct rpmsg_driver *rpdrv =to_rpmsg_driver (drv);const struct rpmsg_device_id *ids = rpdrv->id_table;unsigned int i; +int ret; -if (rpdev->driver_override) -return !strcmp (rpdev->driver_override, drv->name); + ret =device_match_driver_override (dev, drv); +if (ret >=0 ) +return ret;if (ids)for (i =0 ; ids[i].name[0 ]; i++)@@ -535,6 +509,7 @@ static const struct bus_type rpmsg_bus = { .name = "rpmsg", .match = rpmsg_dev_match, .dev_groups = rpmsg_dev_groups, + .driver_override = true, .uevent = rpmsg_uevent, .probe = rpmsg_dev_probe, .remove = rpmsg_dev_remove,@@ -560,11 +535,9 @@ int rpmsg_register_device_override(struct rpmsg_device *rpdev,device_initialize (dev);if (driver_override) { - ret =driver_set_override (dev, &rpdev->driver_override, - driver_override, -strlen (driver_override)); + ret =device_set_driver_override (dev, driver_override);if (ret) { -dev_err (dev,"device_set_override failed: %d\n" , ret); +dev_err (dev,"device_set_driver_override() failed: %d\n" , ret);put_device (dev);return ret; }@@ -573,8 +546,6 @@ int rpmsg_register_device_override(struct rpmsg_device *rpdev, ret =device_add (dev);if (ret) {dev_err (dev,"device_add failed: %d\n" , ret); -kfree (rpdev->driver_override); - rpdev->driver_override =NULL ;put_device (dev); }drivers/rpmsg/virtio_rpmsg_bus.c +0
-1@@ -373,7 +373,6 @@ static void virtio_rpmsg_release_device(struct device *dev)struct rpmsg_device *rpdev =to_rpmsg_device (dev);struct virtio_rpmsg_channel *vch =to_virtio_rpmsg_channel (rpdev); -kfree (rpdev->driver_override);kfree (vch); }include/linux/rpmsg.h +0
-5@@ -41,9 +41,6 @@ struct rpmsg_channel_info {* rpmsg_device - device that belong to the rpmsg bus * @dev: the device struct * @id: device id (used to match between rpmsg drivers and devices) -* @driver_override: driver name to force a match; do not set directly, -* because core frees it; use driver_set_override() to -* set or clear it. * @src: local address * @dst: destination address * @ept: the rpmsg endpoint of this channel @@ -53,7 +50,6 @@ struct rpmsg_channel_info {struct rpmsg_device {struct device dev;struct rpmsg_device_id id; -const char *driver_override;u32 src;u32 dst;struct rpmsg_endpoint *ept; --2.53.0
Danilo Krummrich
maintainerPATCH 8/12 vdpa: use generic driver_override infrastructure
In reply to Danilo KrummrichWhen 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/vdpa/vdpa.c | 48 +++++--------------------------------------- include/linux/vdpa.h | 4 ---- 2 files changed, 5 insertions(+), 47 deletions(-)
drivers/vdpa/vdpa.c +5 -43
@@ -67,57 +67,20 @@ static void vdpa_dev_remove(struct device *d)static int vdpa_dev_match (struct device *dev,const struct device_driver *drv) { -struct vdpa_device *vdev =dev_to_vdpa (dev); +int ret;/* Check override first, and if set, only use the named driver */ -if (vdev->driver_override) -return strcmp (vdev->driver_override, drv->name) ==0 ; + ret =device_match_driver_override (dev, drv); +if (ret >=0 ) +return ret;/* Currently devices must be supported by all vDPA bus drivers */ return 1 ; } -static ssize_t driver_override_store (struct device *dev, -struct device_attribute *attr, -const char *buf,size_t count) -{ -struct vdpa_device *vdev =dev_to_vdpa (dev); -int ret; - - ret =driver_set_override (dev, &vdev->driver_override, buf, count); -if (ret) -return ret; - -return count; -} - -static ssize_t driver_override_show (struct device *dev, -struct device_attribute *attr,char *buf) -{ -struct vdpa_device *vdev =dev_to_vdpa (dev); -ssize_t len; - -device_lock (dev); - len =sysfs_emit (buf,"%s\n" , vdev->driver_override); -device_unlock (dev); - -return len; -} -static DEVICE_ATTR_RW (driver_override); - -static struct attribute *vdpa_dev_attrs[] = { - &dev_attr_driver_override.attr, -NULL , -}; - -static const struct attribute_group vdpa_dev_group = { - .attrs = vdpa_dev_attrs, -}; -__ATTRIBUTE_GROUPS (vdpa_dev); -static const struct bus_type vdpa_bus = { .name ="vdpa" , - .dev_groups = vdpa_dev_groups, + .driver_override = true, .match = vdpa_dev_match, .probe = vdpa_dev_probe, .remove = vdpa_dev_remove,
@@ -132,7 +95,6 @@ static void vdpa_release_dev(struct device *d) ops->free (vdev);ida_free (&vdpa_index_ida, vdev->index); -kfree (vdev->driver_override);kfree (vdev); }
include/linux/vdpa.h +0 -4
@@ -72,9 +72,6 @@ struct vdpa_mgmt_dev;* struct vdpa_device - representation of a vDPA device * @dev: underlying device * @vmap: the metadata passed to upper layer to be used for mapping -* @driver_override: driver name to force a match; do not set directly, -* because core frees it; use driver_set_override() to -* set or clear it. * @config: the configuration ops for this device. * @map: the map ops for this device * @cf_lock: Protects get and set access to configuration layout.
@@ -90,7 +87,6 @@ struct vdpa_mgmt_dev;struct vdpa_device {struct device dev;union virtio_map vmap; -const char *driver_override;const struct vdpa_config_ops *config;const struct virtio_map_ops *map;struct rw_semaphore cf_lock;/* Protects get/set config */
Signature · 1 line
2.53.0
Re: [PATCH 08/12] vdpa: use generic driver_override infrastructure
Quoted 16 lines · Danilo Krummrich
When a driver is probed through
__driver_attach(), the bus'match()callback is called without the device lock held, thus accessing thedriver_overridefield without a lock, which can cause a UAF.Fix this by using the driver-core
driver_overrideinfrastructure taking care of proper locking internally.Note that calling
match()from__driver_attach()without the device lock held is intentional. [1]Reported-by Gui-Dong Han <[email protected]>Fixes 539fec78edb4 ("vdpa: add driver_override support")Signed-off-by Danilo Krummrich <[email protected]>
Quoted 5 lines
--- drivers/vdpa/vdpa.c | 48 +++++--------------------------------------- include/linux/vdpa.h | 4 ---- 2 files changed, 5 insertions(+), 47 deletions(-)
Consolidate this logic is great, thanks!
Quoted 10 lines
drivers/vdpa/vdpa.c +1
-1@@ -67,57 +67,20 @@ static void vdpa_dev_remove(struct device *d)static int vdpa_dev_match (struct device *dev,const struct device_driver *drv) { -struct vdpa_device *vdev =dev_to_vdpa (dev); +int ret;
drivers/vdpa/vdpa.c static void vdpa_dev_remove(struct device *d) · line 78
Quoted 10 lines
drivers/vdpa/vdpa.c +3
-2/* Check override first, and if set, only use the named driver */ -if (vdev->driver_override) -return strcmp (vdev->driver_override, drv->name) ==0 ; + ret =device_match_driver_override (dev, drv); +if (ret >=0 ) +return ret;/* Currently devices must be supported by all vDPA bus drivers */ return 1 ;
Nit: Maybe all of this can be replaced by abs(device_match_driver_override(dev,drv))? Or maybe we're putting too much in the same line.
Either way,
Thanks!
Quoted 80 lines
drivers/vdpa/vdpa.c +1
-40} -static ssize_t driver_override_store (struct device *dev, -struct device_attribute *attr, -const char *buf,size_t count) -{ -struct vdpa_device *vdev =dev_to_vdpa (dev); -int ret; - - ret =driver_set_override (dev, &vdev->driver_override, buf, count); -if (ret) -return ret; - -return count; -} - -static ssize_t driver_override_show (struct device *dev, -struct device_attribute *attr,char *buf) -{ -struct vdpa_device *vdev =dev_to_vdpa (dev); -ssize_t len; - -device_lock (dev); - len =sysfs_emit (buf,"%s\n" , vdev->driver_override); -device_unlock (dev); - -return len; -} -static DEVICE_ATTR_RW (driver_override); - -static struct attribute *vdpa_dev_attrs[] = { - &dev_attr_driver_override.attr, -NULL , -}; - -static const struct attribute_group vdpa_dev_group = { - .attrs = vdpa_dev_attrs, -}; -__ATTRIBUTE_GROUPS (vdpa_dev); -static const struct bus_type vdpa_bus = { .name ="vdpa" , - .dev_groups = vdpa_dev_groups, + .driver_override = true, .match = vdpa_dev_match, .probe = vdpa_dev_probe, .remove = vdpa_dev_remove,@@ -132,7 +95,6 @@ static void vdpa_release_dev(struct device *d) ops->free (vdev);ida_free (&vdpa_index_ida, vdev->index); -kfree (vdev->driver_override);kfree (vdev); }include/linux/vdpa.h +0
-5@@ -72,9 +72,6 @@ struct vdpa_mgmt_dev;* struct vdpa_device - representation of a vDPA device * @dev: underlying device * @vmap: the metadata passed to upper layer to be used for mapping -* @driver_override: driver name to force a match; do not set directly, -* because core frees it; use driver_set_override() to -* set or clear it. * @config: the configuration ops for this device. * @map: the map ops for this device * @cf_lock: Protects get and set access to configuration layout. @@ -90,7 +87,6 @@ struct vdpa_mgmt_dev;struct vdpa_device {struct device dev;union virtio_map vmap; -const char *driver_override;const struct vdpa_config_ops *config;const struct virtio_map_ops *map;struct rw_semaphore cf_lock;/* Protects get/set config */ --2.53.0
Danilo Krummrich
maintainerPATCH 9/12 s390/cio: use generic driver_override infrastructure
In reply to Danilo KrummrichWhen 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/s390/cio/cio.h | 5 ----- drivers/s390/cio/css.c | 34 ++++------------------------------ 2 files changed, 4 insertions(+), 35 deletions(-)
drivers/s390/cio/cio.h +0 -5
@@ -103,11 +103,6 @@ struct subchannel {struct work_struct todo_work;struct schib_config config;u64 dma_mask; -/* -* 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; } __attribute__ ((aligned(8))); DECLARE_PER_CPU_ALIGNED(struct irb, cio_irb);
drivers/s390/cio/css.c +4 -30
@@ -159,7 +159,6 @@ static void css_subchannel_release(struct device *dev) sch->config.intparm =0 ;cio_commit_config (sch); -kfree (sch->driver_override);kfree (sch); }
@@ -323,37 +322,9 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,static DEVICE_ATTR_RO (modalias); -static ssize_t driver_override_store (struct device *dev, -struct device_attribute *attr, -const char *buf,size_t count) -{ -struct subchannel *sch =to_subchannel (dev); -int ret; - - ret =driver_set_override (dev, &sch->driver_override, buf, count); -if (ret) -return ret; - -return count; -} - -static ssize_t driver_override_show (struct device *dev, -struct device_attribute *attr,char *buf) -{ -struct subchannel *sch =to_subchannel (dev); -ssize_t len; - -device_lock (dev); - len =sysfs_emit (buf,"%s\n" , sch->driver_override); -device_unlock (dev); -return len; -} -static DEVICE_ATTR_RW (driver_override); -static struct attribute *subch_attrs[] = { &dev_attr_type.attr, &dev_attr_modalias.attr, - &dev_attr_driver_override.attr,NULL , };
@@ -1356,9 +1327,11 @@ static int css_bus_match(struct device *dev, const struct device_driver *drv)struct subchannel *sch =to_subchannel (dev);const struct css_driver *driver =to_cssdriver (drv);struct css_device_id *id; +int ret;/* When driver_override is set, only bind to the matching driver */ -if (sch->driver_override &&strcmp (sch->driver_override, drv->name)) + ret =device_match_driver_override (dev, drv); +if (ret ==0 )return 0 ;for (id = driver->subchannel_type; id->match_flags; id++) {
@@ -1415,6 +1388,7 @@ static int css_uevent(const struct device *dev, struct kobj_uevent_env *env)static const struct bus_type css_bus_type = { .name ="css" , + .driver_override = true, .match = css_bus_match, .probe = css_probe, .remove = css_remove,
Signature · 1 line
2.53.0
Re: [PATCH 09/12] s390/cio: use generic driver_override infrastructure
Quoted 10 lines · Danilo Krummrich
When a driver is probed through
__driver_attach(), the bus'match()callback is called without the device lock held, thus accessing thedriver_overridefield without a lock, which can cause a UAF.Fix this by using the driver-core
driver_overrideinfrastructure taking care of proper locking internally.Note that calling
match()from__driver_attach()without the device lock held is intentional. [1]
Quoted 6 lines
Reported-by Gui-Dong Han<[email protected]>Fixes ebc3d1791503 ("s390/cio: introduce driver_override on the css bus")Signed-off-by Danilo Krummrich<[email protected]>---
Thank you Danilo.
Danilo Krummrich
maintainerPATCH 10/12 s390/ap: use generic driver_override infrastructure
In reply to Danilo KrummrichWhen the AP masks are updated via apmask_store() or aqmask_store(), ap_bus_revise_bindings() is called after ap_attr_mutex has been released.
This calls __ap_revise_reserved(), which accesses the driver_override field without holding any lock, racing against a concurrent driver_override_store() that may free the old string, resulting in a potential UAF.
Fix this by using the driver-core driver_override infrastructure, which protects all accesses with an internal spinlock.
Note that unlike most other buses, the AP bus does not check driver_override in its match() callback; the override is checked in ap_device_probe() and __ap_revise_reserved() instead.
Also note that we do not enable the driver_override feature of struct bus_type, as AP - in contrast to most other buses - passes "" to sysfs_emit() when the driver_override pointer is NULL. Thus, printing "\n" instead of "(null)\n".
Additionally, AP has a custom counter that is modified in the corresponding custom driver_override_store().
--- drivers/s390/crypto/ap_bus.c | 34 +++++++++++++++++----------------- drivers/s390/crypto/ap_bus.h | 1 - drivers/s390/crypto/ap_queue.c | 24 ++++++------------------ 3 files changed, 23 insertions(+), 36 deletions(-)
drivers/s390/crypto/ap_bus.c +17 -17
@@ -859,25 +859,24 @@ static int __ap_queue_devices_with_id_unregister(struct device *dev, void *data)static int __ap_revise_reserved (struct device *dev,void *dummy) { -int rc, card, queue, devres, drvres; +int rc, card, queue, devres, drvres, ovrd;if (is_queue_dev (dev)) {struct ap_driver *ap_drv =to_ap_drv (dev->driver);struct ap_queue *aq =to_ap_queue (dev); -struct ap_device *ap_dev = &aq->ap_dev; card =AP_QID_CARD (aq->qid); queue =AP_QID_QUEUE (aq->qid); -if (ap_dev->driver_override) { -if (strcmp (ap_dev->driver_override, - ap_drv->driver.name)) { -pr_debug ("reprobing queue=%02x.%04x\n" , card, queue); - rc =device_reprobe (dev); -if (rc) { -AP_DBF_WARN ("%s reprobing queue=%02x.%04x failed\n" , - __func__, card, queue); - } + ovrd =device_match_driver_override (dev, &ap_drv->driver); +if (ovrd >0 ) { +/* override set and matches, nothing to do */ + }else if (ovrd ==0 ) { +pr_debug ("reprobing queue=%02x.%04x\n" , card, queue); + rc =device_reprobe (dev); +if (rc) { +AP_DBF_WARN ("%s reprobing queue=%02x.%04x failed\n" , + __func__, card, queue); } }else {mutex_lock (&ap_attr_mutex);
@@ -928,7 +927,7 @@ int ap_owned_by_def_drv(int card, int queue)if (aq) {const struct device_driver *drv = aq->ap_dev.device.driver;const struct ap_driver *ap_drv =to_ap_drv (drv); -bool override = !!aq->ap_dev.driver_override; +bool override =device_has_driver_override (&aq->ap_dev.device);if (override && drv && ap_drv->flags &AP_DRIVER_FLAG_DEFAULT ) rc =1 ;
@@ -977,7 +976,7 @@ static int ap_device_probe(struct device *dev) {struct ap_device *ap_dev =to_ap_dev (dev);struct ap_driver *ap_drv =to_ap_drv (dev->driver); -int card, queue, devres, drvres, rc = -ENODEV ; +int card, queue, devres, drvres, rc = -ENODEV , ovrd;if (!get_device (dev))return rc;
@@ -991,10 +990,11 @@ static int ap_device_probe(struct device *dev)*/ card =AP_QID_CARD (to_ap_queue (dev)->qid); queue =AP_QID_QUEUE (to_ap_queue (dev)->qid); -if (ap_dev->driver_override) { -if (strcmp (ap_dev->driver_override, - ap_drv->driver.name)) - goto out; + ovrd =device_match_driver_override (dev, &ap_drv->driver); +if (ovrd >0 ) { +/* override set and matches, nothing to do */ + }else if (ovrd ==0 ) { + goto out; }else {mutex_lock (&ap_attr_mutex); devres = test_bit_inv(card, ap_perms.apm) &&
drivers/s390/crypto/ap_bus.h +0 -1
@@ -166,7 +166,6 @@ void ap_driver_unregister(struct ap_driver *);struct ap_device {struct device device;int device_type;/* AP device type. */ -const char *driver_override; };#define to_ap_dev (x) container_of((x), struct ap_device, device)
drivers/s390/crypto/ap_queue.c +6 -18
@@ -734,26 +734,14 @@ static ssize_t driver_override_show(struct device *dev,struct device_attribute *attr, char *buf) { - struct ap_queue *aq = to_ap_queue(dev); -struct ap_device *ap_dev = &aq->ap_dev; -int rc; - -device_lock (dev); -if (ap_dev->driver_override) - rc =sysfs_emit (buf,"%s\n" , ap_dev->driver_override); -else - rc =sysfs_emit (buf,"\n" ); -device_unlock (dev); - -return rc; + guard(spinlock)(&dev->driver_override.lock); +return sysfs_emit (buf,"%s\n" , dev->driver_override.name ?:"" ); }static ssize_t driver_override_store (struct device *dev,struct device_attribute *attr,const char *buf,size_t count) { -struct ap_queue *aq =to_ap_queue (dev); -struct ap_device *ap_dev = &aq->ap_dev;int rc = -EINVAL ;bool old_value;
@@ -764,13 +752,13 @@ static ssize_t driver_override_store(struct device *dev,if (ap_apmask_aqmask_in_use) goto out; - old_value = ap_dev->driver_override ? true : false; - rc =driver_set_override (dev, &ap_dev->driver_override, buf, count); + old_value =device_has_driver_override (dev); + rc =__device_set_driver_override (dev, buf, count);if (rc) goto out; -if (old_value && !ap_dev->driver_override) +if (old_value && !device_has_driver_override (dev)) --ap_driver_override_ctr; -else if (!old_value && ap_dev->driver_override) +else if (!old_value &&device_has_driver_override (dev)) ++ap_driver_override_ctr; rc = count;
Signature · 1 line
2.53.0
Re: [PATCH 10/12] s390/ap: use generic driver_override infrastructure
Quoted 171 lines · Danilo Krummrich
When the AP masks are updated via
apmask_store()oraqmask_store(),ap_bus_revise_bindings()is called afterap_attr_mutexhas been released.This calls
__ap_revise_reserved(), which accesses thedriver_overridefield without holding any lock, racing against a concurrentdriver_override_store()that may free the old string, resulting in a potential UAF.Fix this by using the driver-core
driver_overrideinfrastructure, which protects all accesses with an internal spinlock.Note that unlike most other buses, the AP bus does not check
driver_overridein itsmatch()callback; the override is checked inap_device_probe()and__ap_revise_reserved()instead.Also note that we do not enable the
driver_overridefeature of structbus_type, as AP - in contrast to most other buses - passes "" tosysfs_emit()when thedriver_overridepointer is NULL. Thus, printing "\n" instead of "(null)\n".Additionally, AP has a custom counter that is modified in the corresponding custom
driver_override_store().Fixes d38a87d7c064 ("s390/ap: Support driver_override for AP queue devices")Signed-off-by Danilo Krummrich <[email protected]>--- drivers/s390/crypto/ap_bus.c | 34 +++++++++++++++++----------------- drivers/s390/crypto/ap_bus.h | 1 - drivers/s390/crypto/ap_queue.c | 24 ++++++------------------ 3 files changed, 23 insertions(+), 36 deletions(-)drivers/s390/crypto/ap_bus.c +10
-11@@ -859,25 +859,24 @@ static int __ap_queue_devices_with_id_unregister(struct device *dev, void *data) static int __ap_revise_reserved(struct device *dev, void *dummy) { - int rc, card, queue, devres, drvres; + int rc, card, queue, devres, drvres, ovrd;if (is_queue_dev (dev)) {struct ap_driver *ap_drv =to_ap_drv (dev->driver);struct ap_queue *aq =to_ap_queue (dev); -struct ap_device *ap_dev = &aq->ap_dev; card =AP_QID_CARD (aq->qid); queue =AP_QID_QUEUE (aq->qid); -if (ap_dev->driver_override) { -if (strcmp (ap_dev->driver_override, - ap_drv->driver.name)) { -pr_debug ("reprobing queue=%02x.%04x\n" , card, queue); - rc =device_reprobe (dev); -if (rc) { -AP_DBF_WARN ("%s reprobing queue=%02x.%04x failed\n" , - __func__, card, queue); - } + ovrd =device_match_driver_override (dev, &ap_drv->driver); +if (ovrd >0 ) { +/* override set and matches, nothing to do */ + }else if (ovrd ==0 ) { +pr_debug ("reprobing queue=%02x.%04x\n" , card, queue); + rc =device_reprobe (dev); +if (rc) { +AP_DBF_WARN ("%s reprobing queue=%02x.%04x failed\n" , + __func__, card, queue); } }else {drivers/s390/crypto/ap_bus.c +7
-6mutex_lock (&ap_attr_mutex);@@ -928,7 +927,7 @@ int ap_owned_by_def_drv(int card, int queue)if (aq) {const struct device_driver *drv = aq->ap_dev.device.driver;const struct ap_driver *ap_drv =to_ap_drv (drv); -bool override = !!aq->ap_dev.driver_override; +bool override =device_has_driver_override (&aq->ap_dev.device);if (override && drv && ap_drv->flags &AP_DRIVER_FLAG_DEFAULT ) rc =1 ;@@ -977,7 +976,7 @@ static int ap_device_probe(struct device *dev) {struct ap_device *ap_dev =to_ap_dev (dev);struct ap_driver *ap_drv =to_ap_drv (dev->driver); -int card, queue, devres, drvres, rc = -ENODEV ; +int card, queue, devres, drvres, rc = -ENODEV , ovrd;if (!get_device (dev))return rc;@@ -991,10 +990,11 @@ static int ap_device_probe(struct device *dev)*/ card =AP_QID_CARD (to_ap_queue (dev)->qid); queue =AP_QID_QUEUE (to_ap_queue (dev)->qid); -if (ap_dev->driver_override) { -if (strcmp (ap_dev->driver_override, - ap_drv->driver.name)) - goto out; + ovrd =device_match_driver_override (dev, &ap_drv->driver); +if (ovrd >0 ) { +/* override set and matches, nothing to do */ + }else if (ovrd ==0 ) { + goto out; }else {mutex_lock (&ap_attr_mutex); devres = test_bit_inv(card, ap_perms.apm) &&drivers/s390/crypto/ap_bus.h +0
-1@@ -166,7 +166,6 @@ void ap_driver_unregister(struct ap_driver *);struct ap_device {struct device device;int device_type;/* AP device type. */ -const char *driver_override; };#define to_ap_dev (x) container_of((x), struct ap_device, device)drivers/s390/crypto/ap_queue.c +6
-18@@ -734,26 +734,14 @@ static ssize_t driver_override_show(struct device *dev, struct device_attribute *attr, char *buf) { -struct ap_queue *aq =to_ap_queue (dev); -struct ap_device *ap_dev = &aq->ap_dev; -int rc; - -device_lock (dev); -if (ap_dev->driver_override) - rc =sysfs_emit (buf,"%s\n" , ap_dev->driver_override); -else - rc =sysfs_emit (buf,"\n" ); -device_unlock (dev); - -return rc; +guard (spinlock)(&dev->driver_override.lock); +return sysfs_emit (buf,"%s\n" , dev->driver_override.name ?:"" ); }static ssize_t driver_override_store (struct device *dev,struct device_attribute *attr,const char *buf,size_t count) { -struct ap_queue *aq =to_ap_queue (dev); -struct ap_device *ap_dev = &aq->ap_dev;int rc = -EINVAL ;bool old_value;@@ -764,13 +752,13 @@ static ssize_t driver_override_store(structdevice *dev, if (ap_apmask_aqmask_in_use) goto out; - old_value = ap_dev->driver_override ? true : false; - rc =driver_set_override (dev, &ap_dev->driver_override, buf, count); + old_value =device_has_driver_override (dev); + rc =__device_set_driver_override (dev, buf, count);if (rc) goto out; -if (old_value && !ap_dev->driver_override) +if (old_value && !device_has_driver_override (dev)) --ap_driver_override_ctr; -else if (!old_value && ap_dev->driver_override) +else if (!old_value &&device_has_driver_override (dev)) ++ap_driver_override_ctr;
Quoted 1 line
rc = count;
Thanks Danilo
Re: [PATCH 10/12] s390/ap: use generic driver_override infrastructure
In reply to Danilo KrummrichQuoted 21 lines · Danilo Krummrich
When the AP masks are updated via
apmask_store()oraqmask_store(),ap_bus_revise_bindings()is called afterap_attr_mutexhas been released.This calls
__ap_revise_reserved(), which accesses thedriver_overridefield without holding any lock, racing against a concurrentdriver_override_store()that may free the old string, resulting in a potential UAF.Fix this by using the driver-core
driver_overrideinfrastructure, which protects all accesses with an internal spinlock.Note that unlike most other buses, the AP bus does not check
driver_overridein itsmatch()callback; the override is checked inap_device_probe()and__ap_revise_reserved()instead.Also note that we do not enable the
driver_overridefeature of structbus_type, as AP - in contrast to most other buses - passes "" tosysfs_emit()when thedriver_overridepointer is NULL. Thus, printing "\n" instead of "(null)\n".
Quoted 5 lines
Additionally, AP has a custom counter that is modified in the corresponding custom
driver_override_store().Fixes d38a87d7c064 ("s390/ap: Support driver_override for AP queue devices")Signed-off-by Danilo Krummrich <[email protected]>
Signature · 5 lines
Mit freundlichen Grüßen / Kind regards Holger Dengler -- IBM Systems, Linux on IBM Z Development [email protected]
Danilo Krummrich
maintainerPATCH 11/12 spi: use generic driver_override infrastructure
In reply to Danilo KrummrichWhen 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]
Also note that we do not enable the driver_override feature of struct bus_type, as SPI - in contrast to most other buses - passes "" to sysfs_emit() when the driver_override pointer is NULL. Thus, printing "\n" instead of "(null)\n".
--- drivers/spi/spi.c | 19 +++++++------------ include/linux/spi/spi.h | 5 ----- 2 files changed, 7 insertions(+), 17 deletions(-)
drivers/spi/spi.c +7 -12
@@ -50,7 +50,6 @@ static void spidev_release(struct device *dev)struct spi_device *spi =to_spi_device (dev);spi_controller_put (spi->controller); -kfree (spi->driver_override);free_percpu (spi->pcpu_statistics);kfree (spi); }
@@ -73,10 +72,9 @@ static ssize_t driver_override_store(struct device *dev,struct device_attribute *a, const char *buf, size_t count) { - struct spi_device *spi = to_spi_device(dev); int ret; - ret =driver_set_override (dev, &spi->driver_override, buf, count); + ret =__device_set_driver_override (dev, buf, count);if (ret)return ret;
@@ -86,13 +84,8 @@ static ssize_t driver_override_store(struct device *dev,static ssize_t driver_override_show (struct device *dev,struct device_attribute *a,char *buf) { -const struct spi_device *spi =to_spi_device (dev); -ssize_t len; - -device_lock (dev); - len =sysfs_emit (buf,"%s\n" , spi->driver_override ? :"" ); -device_unlock (dev); -return len; +guard (spinlock)(&dev->driver_override.lock); +return sysfs_emit (buf,"%s\n" , dev->driver_override.name ?:"" ); }static DEVICE_ATTR_RW (driver_override);
@@ -376,10 +369,12 @@ static int spi_match_device(struct device *dev, const struct device_driver *drv) {const struct spi_device *spi =to_spi_device (dev);const struct spi_driver *sdrv =to_spi_driver (drv); +int ret;/* Check override first, and if set, only use the named driver */ -if (spi->driver_override) -return strcmp (spi->driver_override, drv->name) ==0 ; + ret =device_match_driver_override (dev, drv); +if (ret >=0 ) +return ret;/* Attempt an OF style match */ if (of_driver_match_device (dev, drv))
include/linux/spi/spi.h +0 -5
@@ -159,10 +159,6 @@ extern void spi_transfer_cs_change_delay_exec(struct spi_message *msg,* @modalias: Name of the driver to use with this device, or an alias * for that name. This appears in the sysfs "modalias" attribute * for driver coldplugging, and in uevents used for hotplugging -* @driver_override: If the name of a driver is written to this attribute, then -* the device will bind to the named driver and only the named driver. -* Do not set directly, because core frees it; use driver_set_override() to -* set or clear it. * @pcpu_statistics: statistics for the spi_device * @word_delay: delay to be inserted between consecutive * words of a transfer
@@ -224,7 +220,6 @@ struct spi_device {void *controller_state;void *controller_data;char modalias[SPI_NAME_SIZE ]; -const char *driver_override;/* The statistics */ struct spi_statistics __percpu *pcpu_statistics;
Signature · 1 line
2.53.0
Danilo Krummrich
maintainerPATCH 12/12 driver core: remove driver_set_override()
In reply to Danilo KrummrichAll buses have been converted from driver_set_override() to the generic driver_override infrastructure introduced in commit cb3d1049f4ea ("driver core: generalize driver_override in struct device").
Buses now either opt into the generic sysfs callbacks via the bus_type::driver_override flag, or use device_set_driver_override() / __device_set_driver_override() directly.
Thus, remove the now-unused driver_set_override() helper.
--- drivers/base/driver.c | 75 ----------------------------------- include/linux/device/driver.h | 2 - 2 files changed, 77 deletions(-)
drivers/base/driver.c +0 -75
@@ -30,81 +30,6 @@ static struct device *next_device(struct klist_iter *i)return dev; } -/** -* driver_set_override() - Helper to set or clear driver override. -* @dev: Device to change -* @override: Address of string to change (e.g. &device->driver_override); -* The contents will be freed and hold newly allocated override. -* @s: NUL-terminated string, new driver name to force a match, pass empty -* string to clear it ("" or "\n", where the latter is only for sysfs -* interface). -* @len: length of @s -* -* Helper to set or clear driver override in a device, intended for the cases -* when the driver_override field is allocated by driver/bus code. -* -* Returns: 0 on success or a negative error code on failure. -*/ -int driver_set_override (struct device *dev,const char **override, -const char *s,size_t len) -{ -const char *new, *old; -char *cp; - -if (!override || !s) -return -EINVAL ; - -/* -* The stored value will be used in sysfs show callback (sysfs_emit()), -* which has a length limit of PAGE_SIZE and adds a trailing newline. -* Thus we can store one character less to avoid truncation during sysfs -* show. -*/ -if (len >= (PAGE_SIZE -1 )) -return -EINVAL ; - -/* -* Compute the real length of the string in case userspace sends us a -* bunch of \0 characters like python likes to do. -*/ - len =strlen (s); - -if (!len) { -/* Empty string passed - clear override */ -device_lock (dev); - old = *override; - *override =NULL ; -device_unlock (dev); -kfree (old); - -return 0 ; - } - - cp =strnchr (s, len,'\n' ); -if (cp) - len = cp - s; - - new =kstrndup (s, len,GFP_KERNEL ); -if (!new) -return -ENOMEM ; - -device_lock (dev); - old = *override; -if (cp != s) { - *override = new; - }else { -/* "\n" passed - clear override */ -kfree (new); - *override =NULL ; - } -device_unlock (dev); - -kfree (old); - -return 0 ; -} -EXPORT_SYMBOL_GPL (driver_set_override); -/** * driver_for_each_device - Iterator for devices bound to a driver. * @drv: Driver we're iterating.
include/linux/device/driver.h +0 -2
@@ -160,8 +160,6 @@ int __must_check driver_create_file(const struct device_driver *driver,void driver_remove_file (const struct device_driver *driver,const struct driver_attribute *attr); -int driver_set_override (struct device *dev,const char **override, -const char *s,size_t len);int __must_check driver_for_each_device (struct device_driver *drv,struct device *start,void *data,device_iter_t fn); struct device *driver_find_device(const struct device_driver *drv,
Signature · 1 line
2.53.0
Re: [PATCH 12/12] driver core: remove driver_set_override()
Quoted 8 lines · Danilo Krummrich
All buses have been converted from
driver_set_override()to the genericdriver_overrideinfrastructure introduced in commitcb3d1049f4ea("driver core: generalizedriver_overridein struct device").Buses now either opt into the generic sysfs callbacks via the
bus_type::driver_overrideflag, or usedevice_set_driver_override()/__device_set_driver_override()directly.
Quoted 8 lines
Thus, remove the now-unused
driver_set_override()helper.Signed-off-by Danilo Krummrich <[email protected]>--- drivers/base/driver.c | 75 ----------------------------------- include/linux/device/driver.h | 2 - 2 files changed, 77 deletions(-)
Mark Brown
maintainerRe: (subset) [PATCH 00/12] treewide: Convert buses to use generic driver_override
In reply to Danilo KrummrichQuoted 11 lines · Danilo Krummrich
treewide: Convert buses to use generic
driver_overrideThis is the follow-up of the
driver_overridegeneralization in [1], converting the remaining 11 busses and removing the now-unuseddriver_set_override()helper.All of them (except AP, which has a different race condition) are prone to the potential UAF described in [2], caused by accessing the
driver_overridefield from their correspondingmatch()callback.[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-7.0
Thanks!
[11/12] spi: use generic driver_override infrastructure
https://git.kernel.org/broonie/spi/c/cc34d77dd487
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks,
Mark
Quoted 32 lines · Danilo Krummrich
This is the follow-up of the
driver_overridegeneralization in [1], converting the remaining 11 busses and removing the now-unuseddriver_set_override()helper.All of them (except AP, which has a different race condition) are prone to the potential UAF described in [2], caused by accessing the
driver_overridefield from their correspondingmatch()callback.In order to address this, the generalized
driver_overridefield in struct device is protected with a spinlock. The driver-core provides accessors, such asdevice_match_driver_override(),device_has_driver_override()anddevice_set_driver_override(), which all ensure proper locking internally.Additionally, the driver-core provides a
driver_overrideflag in structbus_type, which, once enabled, automatically registers generic sysfs callbacks, allowing userspace to modify thedriver_overridefield.SPI and AP are a bit special; both print "\n" when
driver_overrideis not set, whereas all other buses (and thus the driver-core) produce "(null)\n" in this case.Hence, SPI and AP do not take advantage of the
driver_overrideflag in structbus_type; AP additionally maintains a counter in its custom sysfsstore().Technically, we could support a custom fallback string when
driver_overrideis unset in structbus_type, but only SPI would benefit from this, since AP has additional custom logic instore()anyways.(I'm not sure if there are userspace programs that strictly rely on this; driverctl seems to check for both, but I rather not break some userspace tool I'm not aware of. :)
Quoted 6 lines
This series is based on v7.0-rc5 with no additional dependencies, hence those patches can be picked up by subsystems individually.
[1] https://lore.kernel.org/driver-core/[email protected]/
[2] https://bugzilla.kernel.org/show_bug.cgi?id=220789
[3] https://gitlab.com/driverctl/driverctl/-/blob/0.121/driverctl?ref_type=tags#L99
vdpa bits:
I assume it'll all be merged together?
Quoted 52 lines
Danilo Krummrich (12):
amba: use genericdriver_overrideinfrastructure
bus: fsl-mc: use genericdriver_overrideinfrastructure
cdx: use genericdriver_overrideinfrastructure
hv: vmbus: use genericdriver_overrideinfrastructure
PCI: use genericdriver_overrideinfrastructure platform/wmi: use genericdriver_overrideinfrastructure
rpmsg: use genericdriver_overrideinfrastructure
vdpa: use genericdriver_overrideinfrastructure s390/cio: use genericdriver_overrideinfrastructure s390/ap: use genericdriver_overrideinfrastructure
spi: use genericdriver_overrideinfrastructure driver core: removedriver_set_override()drivers/amba/bus.c | 37 +++------------ drivers/base/driver.c | 75 ------------------------------ drivers/bus/fsl-mc/fsl-mc-bus.c | 43 +++-------------- drivers/cdx/cdx.c | 40 ++-------------- drivers/hv/vmbus_drv.c | 36 ++------------ drivers/pci/pci-driver.c | 11 +++-- drivers/pci/pci-sysfs.c | 28 ----------- drivers/pci/probe.c | 1 - drivers/platform/wmi/core.c | 36 ++------------ drivers/rpmsg/qcom_glink_native.c | 2 - drivers/rpmsg/rpmsg_core.c | 43 +++-------------- drivers/rpmsg/virtio_rpmsg_bus.c | 1 - drivers/s390/cio/cio.h | 5 -- drivers/s390/cio/css.c | 34 ++------------ drivers/s390/crypto/ap_bus.c | 34 +++++++------- drivers/s390/crypto/ap_bus.h | 1 - drivers/s390/crypto/ap_queue.c | 24 +++------- drivers/spi/spi.c | 19 +++----- drivers/vdpa/vdpa.c | 48 ++----------------- drivers/vfio/fsl-mc/vfio_fsl_mc.c | 4 +- drivers/vfio/pci/vfio_pci_core.c | 5 +- drivers/xen/xen-pciback/pci_stub.c | 6 ++- include/linux/amba/bus.h | 5 -- include/linux/cdx/cdx_bus.h | 4 -- include/linux/device/driver.h | 2 - include/linux/fsl/mc.h | 4 -- include/linux/hyperv.h | 5 -- include/linux/pci.h | 6 --- include/linux/rpmsg.h | 4 -- include/linux/spi/spi.h | 5 -- include/linux/vdpa.h | 4 -- include/linux/wmi.h | 4 -- 32 files changed, 88 insertions(+), 488 deletions(-)base-commit:
c369299895a591d96745d6492d4888259b004a9eSignature · 1 line
2.53.0
Danilo Krummrich
maintainerQuoted 5 lines · Michael S. Tsirkin
vdpa bits:
I assume it'll all be merged together?
I can take it through the driver-core tree if you prefer, but you can also pick it up yourself.
Danilo Krummrich
maintainerRe: (subset) [PATCH 00/12] treewide: Convert buses to use generic driver_override
In reply to Danilo KrummrichQuoted 6 lines · Danilo Krummrich
Danilo Krummrich (12):
PCI: use genericdriver_overrideinfrastructure platform/wmi: use genericdriver_overrideinfrastructure
vdpa: use genericdriver_overrideinfrastructure s390/cio: use genericdriver_overrideinfrastructure s390/ap: use genericdriver_overrideinfrastructure
Applied to driver-core-testing, thanks!
Quoted 4 lines
amba: use generic
driver_overrideinfrastructure
cdx: use genericdriver_overrideinfrastructure
hv: vmbus: use genericdriver_overrideinfrastructure
rpmsg: use genericdriver_overrideinfrastructure
I have not picked these up, as they have not received ACKs from the corresponding subsystem maintainers so far.
Quoted 2 lines
bus: fsl-mc: use generic
driver_overrideinfrastructure
spi: use genericdriver_overrideinfrastructure
These have already been picked up via the respective subsystem trees -- thanks!
Thanks,
Danilo
Christophe Leroy (CS GROUP)
maintainerRe: (subset) [PATCH 00/12] treewide: Convert buses to use generic driver_override
Le 04/04/2026 à 17:07, Danilo Krummrich a écrit :
Quoted 10 lines · Danilo Krummrich
Quoted 6 lines
Danilo Krummrich (12):
PCI: use genericdriver_overrideinfrastructure platform/wmi: use genericdriver_overrideinfrastructure
vdpa: use genericdriver_overrideinfrastructure s390/cio: use genericdriver_overrideinfrastructure s390/ap: use genericdriver_overrideinfrastructureApplied to driver-core-testing, thanks!
Quoted 9 lines
Quoted 4 lines
amba: use generic
driver_overrideinfrastructure
cdx: use genericdriver_overrideinfrastructure
hv: vmbus: use genericdriver_overrideinfrastructure
rpmsg: use genericdriver_overrideinfrastructureI have not picked these up, as they have not received ACKs from the corresponding subsystem maintainers so far.
Quoted 1 line
bus: fsl-mc: use generic
driver_overrideinfrastructure
I droped it from soc_fsl tree, some dependency must be missing.
Feal free to take it if you can, it is acked-by Ioana.
Quoted 6 lines
Quoted 1 line
spi: use generic
driver_overrideinfrastructureThese have already been picked up via the respective subsystem trees -- thanks!
Thanks,
Danilo
Danilo Krummrich
maintainerRe: (subset) [PATCH 00/12] treewide: Convert buses to use generic driver_override
Quoted 18 lines · Christophe Leroy (CS GROUP)
Le 04/04/2026 à 17:07, Danilo Krummrich a écrit :
Quoted 15 lines · Danilo Krummrich
Quoted 6 lines
Danilo Krummrich (12):
PCI: use genericdriver_overrideinfrastructure platform/wmi: use genericdriver_overrideinfrastructure
vdpa: use genericdriver_overrideinfrastructure s390/cio: use genericdriver_overrideinfrastructure s390/ap: use genericdriver_overrideinfrastructureApplied to driver-core-testing, thanks!
Quoted 4 lines
amba: use generic
driver_overrideinfrastructure
cdx: use genericdriver_overrideinfrastructure
hv: vmbus: use genericdriver_overrideinfrastructure
rpmsg: use genericdriver_overrideinfrastructure
Quoted 8 lines
Quoted 4 lines
I have not picked these up, as they have not received ACKs from the corresponding subsystem maintainers so far.
Quoted 1 line
bus: fsl-mc: use generic
driver_overrideinfrastructureI droped it from
soc_fsltree, some dependency must be missing.Feal free to take it if you can, it is acked-by Ioana.
It is based on v7.0-rc5; if you want I can pick it up.
Quoted 6 lines
Quoted 6 lines
Quoted 1 line
spi: use generic
driver_overrideinfrastructureThese have already been picked up via the respective subsystem trees -- thanks!
Thanks,
Danilo
Christophe Leroy (CS GROUP)
maintainerRe: (subset) [PATCH 00/12] treewide: Convert buses to use generic driver_override
Le 04/04/2026 à 19:04, Danilo Krummrich a écrit :
Quoted 22 lines · Christophe Leroy (CS GROUP)
Quoted 21 lines
Le 04/04/2026 à 17:07, Danilo Krummrich a écrit :
Quoted 18 lines · Danilo Krummrich
Quoted 6 lines
Danilo Krummrich (12):
PCI: use genericdriver_overrideinfrastructure platform/wmi: use genericdriver_overrideinfrastructure
vdpa: use genericdriver_overrideinfrastructure s390/cio: use genericdriver_overrideinfrastructure s390/ap: use genericdriver_overrideinfrastructureApplied to driver-core-testing, thanks!
Quoted 4 lines
amba: use generic
driver_overrideinfrastructure
cdx: use genericdriver_overrideinfrastructure
hv: vmbus: use genericdriver_overrideinfrastructure
rpmsg: use genericdriver_overrideinfrastructureI have not picked these up, as they have not received ACKs from the corresponding subsystem maintainers so far.
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
Quoted 1 line
bus: fsl-mc: use generic
driver_overrideinfrastructureI droped it from
soc_fsltree, some dependency must be missing.Feal free to take it if you can, it is acked-by Ioana.
It is based on v7.0-rc5; if you want I can pick it up.
Yes please pick it up as my tree is based on rc1.
Thanks
Christophe
Quoted 8 lines
Quoted 7 lines
Quoted 7 lines
Quoted 2 lines
spi: use generic
driver_overrideinfrastructureThese have already been picked up via the respective subsystem trees -- thanks!
Thanks,
Danilo
Danilo Krummrich
maintainerRe: (subset) [PATCH 00/12] treewide: Convert buses to use generic driver_override
Quoted 1 line · Christophe Leroy (CS GROUP)
Yes please pick it up as my tree is based on rc1.
Applied the patch to driver-core-testing, thanks!