* Re: [PATCH 1/3] driver core: generalize driver_override in struct device
2026-03-02 0:25 ` [PATCH 1/3] driver core: generalize driver_override in struct device Danilo Krummrich
@ 2026-03-02 7:35 ` Gui-Dong Han
2026-03-02 8:36 ` Gui-Dong Han
` (2 subsequent siblings)
3 siblings, 0 replies; 21+ messages in thread
From: Gui-Dong Han @ 2026-03-02 7:35 UTC (permalink / raw)
To: Danilo Krummrich
Cc: gregkh, rafael, ysato, dalias, glaubitz, abelvesa, srini,
s.nawrocki, nuno.sa, driver-core, linux-kernel, imx, linux-hwmon,
linux-arm-msm, linux-sound, linux-sh, Wang Jiayue
On Mon, Mar 2, 2026 at 8:27 AM Danilo Krummrich <dakr@kernel.org> wrote:
>
> Currently, there are 12 busses (including platform and PCI) that
> duplicate the driver_override logic for their individual devices.
>
> All of them seem to be prone to the bug described in [1].
>
> While this could be solved for every bus individually using a separate
> lock, solving this in the driver-core generically results in less (and
> cleaner) changes overall.
>
> Thus, move driver_override to struct device, provide corresponding
> accessors for busses and handle locking with a separate lock internally.
>
> In particular, add device_set_driver_override(),
> device_has_driver_override(), device_match_driver_override() and a
> helper, DEVICE_ATTR_DRIVER_OVERRIDE(), to declare the corresponding
> sysfs store() and show() callbacks.
>
> Until all busses have migrated, keep driver_set_override() in place.
>
> Note that we can't use the device lock for the reasons described in [2].
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=220789 [1]
> Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/ [2]
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
Hi Danilo,
I wanted to test if this fixes the issue using PoCs, but I'm hitting a
KASAN splat right during boot. The issue disappears if I revert this
patch.
KASAN report:
[ 7.266874] ==================================================================
[ 7.267707] BUG: KASAN: slab-use-after-free in device_release+0x1f4/0x240
[ 7.267707] Read of size 8 at addr ffff888003f4a370 by task kworker/1:0/24
[ 7.267707]
[ 7.267707] CPU: 1 UID: 0 PID: 24 Comm: kworker/1:0 Not tainted
7.0.0-rc2-00001-gc1a10dc76109 #4 PREEMP
[ 7.267707] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX,
arch_caps fix, 1996), BIOS 1.16.3-de4
[ 7.267707] Workqueue: events_long serio_handle_event
[ 7.267707] Call Trace:
[ 7.267707] <TASK>
[ 7.267707] dump_stack_lvl+0x66/0xa0
[ 7.267707] print_report+0xce/0x660
[ 7.267707] ? device_release+0x1f4/0x240
[ 7.267707] ? __virt_addr_valid+0x208/0x410
[ 7.267707] ? device_release+0x1f4/0x240
[ 7.267707] kasan_report+0xe0/0x110
[ 7.267707] ? device_release+0x1f4/0x240
[ 7.267707] device_release+0x1f4/0x240
[ 7.267707] kobject_put+0x1c8/0x450
[ 7.267707] atkbd_connect+0x615/0x9e0
[ 7.267707] ? __pfx_atkbd_connect+0x10/0x10
[ 7.267707] ? kernfs_create_link+0x169/0x230
[ 7.267707] ? do_raw_spin_unlock+0x53/0x220
[ 7.267707] serio_driver_probe+0x72/0xb0
[ 7.267707] really_probe+0x254/0x910
[ 7.267707] __driver_probe_device+0x20b/0x3d0
[ 7.267707] driver_probe_device+0x45/0x130
[ 7.267707] __driver_attach+0x1f6/0x550
[ 7.267707] ? __pfx___driver_attach+0x10/0x10
[ 7.267707] bus_for_each_dev+0x103/0x180
[ 7.267707] ? __pfx_bus_for_each_dev+0x10/0x10
[ 7.267707] ? _raw_spin_unlock_irqrestore+0x3f/0x50
[ 7.267707] ? lockdep_hardirqs_on_prepare+0xea/0x1a0
[ 7.267707] serio_handle_event+0x1ce/0x840
[ 7.267707] process_one_work+0x7fc/0x1760
[ 7.267707] ? __pfx_process_one_work+0x10/0x10
[ 7.267707] ? lock_is_held_type+0x8f/0x100
[ 7.267707] ? __pfx_serio_handle_event+0x10/0x10
[ 7.267707] worker_thread+0x593/0xfb0
[ 7.267707] ? __pfx_worker_thread+0x10/0x10
[ 7.267707] kthread+0x319/0x400
[ 7.267707] ? __pfx_kthread+0x10/0x10
[ 7.267707] ret_from_fork+0x590/0x830
[ 7.267707] ? __pfx_ret_from_fork+0x10/0x10
[ 7.267707] ? __switch_to+0x860/0xe50
[ 7.267707] ? __switch_to_asm+0x39/0x70
[ 7.267707] ? __switch_to_asm+0x33/0x70
[ 7.267707] ? __pfx_kthread+0x10/0x10
[ 7.267707] ret_from_fork_asm+0x1a/0x30
[ 7.267707] </TASK>
[ 7.267707]
[ 7.267707] Allocated by task 24:
[ 7.267707] kasan_save_stack+0x33/0x60
[ 7.267707] kasan_save_track+0x14/0x30
[ 7.267707] __kasan_kmalloc+0x8f/0xa0
[ 7.267707] input_allocate_device+0x3f/0x330
[ 7.267707] atkbd_connect+0x97/0x9e0
[ 7.267707] serio_driver_probe+0x72/0xb0
[ 7.267707] really_probe+0x254/0x910
[ 7.267707] __driver_probe_device+0x20b/0x3d0
[ 7.267707] driver_probe_device+0x45/0x130
[ 7.267707] __driver_attach+0x1f6/0x550
[ 7.267707] bus_for_each_dev+0x103/0x180
[ 7.267707] serio_handle_event+0x1ce/0x840
[ 7.267707] process_one_work+0x7fc/0x1760
[ 7.267707] worker_thread+0x593/0xfb0
[ 7.267707] kthread+0x319/0x400
[ 7.267707] ret_from_fork+0x590/0x830
[ 7.267707] ret_from_fork_asm+0x1a/0x30
[ 7.267707]
[ 7.267707] Freed by task 24:
[ 7.267707] kasan_save_stack+0x33/0x60
[ 7.267707] kasan_save_track+0x14/0x30
[ 7.267707] kasan_save_free_info+0x3b/0x60
[ 7.267707] __kasan_slab_free+0x43/0x70
[ 7.267707] kfree+0x193/0x4f0
[ 7.267707] input_dev_release+0xa6/0xd0
[ 7.267707] device_release+0x9a/0x240
[ 7.267707] kobject_put+0x1c8/0x450
[ 7.267707] atkbd_connect+0x615/0x9e0
[ 7.267707] serio_driver_probe+0x72/0xb0
[ 7.267707] really_probe+0x254/0x910
[ 7.267707] __driver_probe_device+0x20b/0x3d0
[ 7.267707] driver_probe_device+0x45/0x130
[ 7.267707] __driver_attach+0x1f6/0x550
[ 7.267707] bus_for_each_dev+0x103/0x180
[ 7.267707] serio_handle_event+0x1ce/0x840
[ 7.267707] process_one_work+0x7fc/0x1760
[ 7.267707] worker_thread+0x593/0xfb0
[ 7.267707] kthread+0x319/0x400
[ 7.267707] ret_from_fork+0x590/0x830
[ 7.267707] ret_from_fork_asm+0x1a/0x30
[ 7.267707]
[ 7.267707] The buggy address belongs to the object at ffff888003f4a000
[ 7.267707] which belongs to the cache kmalloc-2k of size 2048
[ 7.267707] The buggy address is located 880 bytes inside of
[ 7.267707] freed 2048-byte region [ffff888003f4a000, ffff888003f4a800)
[ 7.267707]
[ 7.267707] The buggy address belongs to the physical page:
[ 7.267707] page: refcount:0 mapcount:0 mapping:0000000000000000
index:0xffff888003f4b800 pfn:0x3f48
[ 7.267707] head: order:3 mapcount:0 entire_mapcount:0
nr_pages_mapped:0 pincount:0
[ 7.267707] flags: 0x100000000000240(workingset|head|node=0|zone=1)
[ 7.267707] page_type: f5(slab)
[ 7.267707] raw: 0100000000000240 ffff888001043240 ffff888001041088
ffff888001041088
[ 7.267707] raw: ffff888003f4b800 0000000000050002 00000000f5000000
0000000000000000
[ 7.267707] head: 0100000000000240 ffff888001043240
ffff888001041088 ffff888001041088
[ 7.267707] head: ffff888003f4b800 0000000000050002
00000000f5000000 0000000000000000
[ 7.267707] head: 0100000000000003 ffffea00000fd201
00000000ffffffff 00000000ffffffff
[ 7.267707] head: 0000000000000000 0000000000000000
00000000ffffffff 0000000000000000
[ 7.267707] page dumped because: kasan: bad access detected
[ 7.267707]
[ 7.267707] Memory state around the buggy address:
[ 7.267707] ffff888003f4a200: fb fb fb fb fb fb fb fb fb fb fb fb
fb fb fb fb
[ 7.267707] ffff888003f4a280: fb fb fb fb fb fb fb fb fb fb fb fb
fb fb fb fb
[ 7.267707] >ffff888003f4a300: fb fb fb fb fb fb fb fb fb fb fb fb
fb fb fb fb
[ 7.267707] ^
[ 7.267707] ffff888003f4a380: fb fb fb fb fb fb fb fb fb fb fb fb
fb fb fb fb
[ 7.267707] ffff888003f4a400: fb fb fb fb fb fb fb fb fb fb fb fb
fb fb fb fb
[ 7.267707] ==================================================================
[ 7.293685] Disabling lock debugging due to kernel taint
This is on a basic QEMU x86_64 VM. Note that I did not apply the "WIP:
treewide: make callsites use generic driver_override" patch.
I'm currently looking into the root cause.
Thanks.
> ---
> drivers/base/core.c | 2 ++
> drivers/base/dd.c | 60 +++++++++++++++++++++++++++++++
> include/linux/device.h | 81 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 143 insertions(+)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 791f9e444df8..a8cb90577d10 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -2566,6 +2566,7 @@ static void device_release(struct kobject *kobj)
> else
> WARN(1, KERN_ERR "Device '%s' does not have a release() function, it is broken and must be fixed. See Documentation/core-api/kobject.rst.\n",
> dev_name(dev));
> + kfree(dev->driver_override.name);
> kfree(p);
> }
>
> @@ -3159,6 +3160,7 @@ void device_initialize(struct device *dev)
> kobject_init(&dev->kobj, &device_ktype);
> INIT_LIST_HEAD(&dev->dma_pools);
> mutex_init(&dev->mutex);
> + spin_lock_init(&dev->driver_override.lock);
> lockdep_set_novalidate_class(&dev->mutex);
> spin_lock_init(&dev->devres_lock);
> INIT_LIST_HEAD(&dev->devres_head);
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 0354f209529c..697e36e63cab 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -381,6 +381,66 @@ static void __exit deferred_probe_exit(void)
> }
> __exitcall(deferred_probe_exit);
>
> +int __device_set_driver_override(struct device *dev, const char *s, size_t len)
> +{
> + const char *new, *old;
> + char *cp;
> +
> + if (!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 */
> + spin_lock(&dev->driver_override.lock);
> + old = dev->driver_override.name;
> + dev->driver_override.name = NULL;
> + spin_unlock(&dev->driver_override.lock);
> + kfree(old);
> +
> + return 0;
> + }
> +
> + cp = strnchr(s, len, '\n');
> + if (cp)
> + len = cp - s;
> +
> + new = kstrndup(s, len, GFP_KERNEL);
> + if (!new)
> + return -ENOMEM;
> +
> + spin_lock(&dev->driver_override.lock);
> + old = dev->driver_override.name;
> + if (cp != s) {
> + dev->driver_override.name = new;
> + spin_unlock(&dev->driver_override.lock);
> + } else {
> + /* "\n" passed - clear override */
> + dev->driver_override.name = NULL;
> + spin_unlock(&dev->driver_override.lock);
> +
> + kfree(new);
> + }
> + kfree(old);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(__device_set_driver_override);
> +
> /**
> * device_is_bound() - Check if device is bound to a driver
> * @dev: device to check
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 0be95294b6e6..4599156d5cbd 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -266,6 +266,33 @@ ssize_t device_show_string(struct device *dev, struct device_attribute *attr,
> struct dev_ext_attribute dev_attr_##_name = \
> { __ATTR(_name, (_mode) & ~0222, device_show_string, NULL), (_var) }
>
> +/**
> + * DEVICE_ATTR_DRIVER_OVERRIDE - Define sysfs driver_override attribute callbacks
> + *
> + * Generates the standard driver_override_show() and driver_override_store()
> + * sysfs callbacks and the static DEVICE_ATTR_RW(driver_override) declaration.
> + */
> +#define DEVICE_ATTR_DRIVER_OVERRIDE() \
> +static ssize_t driver_override_store(struct device *dev, \
> + struct device_attribute *attr, \
> + const char *buf, size_t count) \
> +{ \
> + int ret; \
> + \
> + ret = __device_set_driver_override(dev, buf, count); \
> + if (ret) \
> + return ret; \
> + \
> + return count; \
> +} \
> +static ssize_t driver_override_show(struct device *dev, \
> + struct device_attribute *attr, char *buf) \
> +{ \
> + guard(spinlock)(&dev->driver_override.lock); \
> + return sysfs_emit(buf, "%s\n", dev->driver_override.name); \
> +} \
> +static DEVICE_ATTR_RW(driver_override)
> +
> #define DEVICE_ATTR_IGNORE_LOCKDEP(_name, _mode, _show, _store) \
> struct device_attribute dev_attr_##_name = \
> __ATTR_IGNORE_LOCKDEP(_name, _mode, _show, _store)
> @@ -483,6 +510,8 @@ struct device_physical_location {
> * on. This shrinks the "Board Support Packages" (BSPs) and
> * minimizes board-specific #ifdefs in drivers.
> * @driver_data: Private pointer for driver specific info.
> + * @driver_override: Driver name to force a match. Do not touch directly; use
> + * device_set_driver_override() instead.
> * @links: Links to suppliers and consumers of this device.
> * @power: For device power management.
> * See Documentation/driver-api/pm/devices.rst for details.
> @@ -576,6 +605,10 @@ struct device {
> core doesn't touch it */
> void *driver_data; /* Driver data, set and get with
> dev_set_drvdata/dev_get_drvdata */
> + struct {
> + const char *name;
> + spinlock_t lock;
> + } driver_override;
> struct mutex mutex; /* mutex to synchronize calls to
> * its driver.
> */
> @@ -701,6 +734,54 @@ struct device_link {
>
> #define kobj_to_dev(__kobj) container_of_const(__kobj, struct device, kobj)
>
> +int __device_set_driver_override(struct device *dev, const char *s, size_t len);
> +
> +/**
> + * device_set_driver_override() - Helper to set or clear driver override.
> + * @dev: Device to change
> + * @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).
> + *
> + * Helper to set or clear driver override of a device.
> + *
> + * Returns: 0 on success or a negative error code on failure.
> + */
> +static inline int device_set_driver_override(struct device *dev, const char *s)
> +{
> + return __device_set_driver_override(dev, s, strlen(s));
> +}
> +
> +/**
> + * device_has_driver_override() - Check if a driver override has been set.
> + * @dev: device to check
> + *
> + * Returns true if a driver override has been set for this device.
> + */
> +static inline bool device_has_driver_override(struct device *dev)
> +{
> + guard(spinlock)(&dev->driver_override.lock);
> + return !!dev->driver_override.name;
> +}
> +
> +/**
> + * device_match_driver_override() - Match a driver against the device's driver_override.
> + * @dev: device to check
> + * @drv: driver to match against
> + *
> + * Returns > 0 if a driver override is set and matches the given driver, 0 if a
> + * driver override is set but does not match, or < 0 if a driver override is not
> + * set at all.
> + */
> +static inline int device_match_driver_override(struct device *dev,
> + const struct device_driver *drv)
> +{
> + guard(spinlock)(&dev->driver_override.lock);
> + if (dev->driver_override.name)
> + return !strcmp(dev->driver_override.name, drv->name);
> + return -1;
> +}
> +
> /**
> * device_iommu_mapped - Returns true when the device DMA is translated
> * by an IOMMU
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/3] driver core: generalize driver_override in struct device
2026-03-02 0:25 ` [PATCH 1/3] driver core: generalize driver_override in struct device Danilo Krummrich
2026-03-02 7:35 ` Gui-Dong Han
@ 2026-03-02 8:36 ` Gui-Dong Han
2026-03-02 10:05 ` Danilo Krummrich
2026-03-02 10:00 ` Geert Uytterhoeven
2026-03-02 10:23 ` Armin Wolf
3 siblings, 1 reply; 21+ messages in thread
From: Gui-Dong Han @ 2026-03-02 8:36 UTC (permalink / raw)
To: Danilo Krummrich
Cc: gregkh, rafael, ysato, dalias, glaubitz, abelvesa, srini,
s.nawrocki, nuno.sa, driver-core, linux-kernel, imx, linux-hwmon,
linux-arm-msm, linux-sound, linux-sh, Wang Jiayue
On Mon, Mar 2, 2026 at 8:27 AM Danilo Krummrich <dakr@kernel.org> wrote:
>
> Currently, there are 12 busses (including platform and PCI) that
> duplicate the driver_override logic for their individual devices.
>
> All of them seem to be prone to the bug described in [1].
>
> While this could be solved for every bus individually using a separate
> lock, solving this in the driver-core generically results in less (and
> cleaner) changes overall.
>
> Thus, move driver_override to struct device, provide corresponding
> accessors for busses and handle locking with a separate lock internally.
>
> In particular, add device_set_driver_override(),
> device_has_driver_override(), device_match_driver_override() and a
> helper, DEVICE_ATTR_DRIVER_OVERRIDE(), to declare the corresponding
> sysfs store() and show() callbacks.
>
> Until all busses have migrated, keep driver_set_override() in place.
>
> Note that we can't use the device lock for the reasons described in [2].
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=220789 [1]
> Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/ [2]
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
Applying the following diff fixes the KASAN issue. It was just a minor bug.
diff --git a/drivers/base/core.c b/drivers/base/core.c
index a8cb90577d10..09b98f02f559 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2556,6 +2556,7 @@ static void device_release(struct kobject *kobj)
devres_release_all(dev);
kfree(dev->dma_range_map);
+ kfree(dev->driver_override.name);
if (dev->release)
dev->release(dev);
@@ -2566,7 +2567,6 @@ static void device_release(struct kobject *kobj)
else
WARN(1, KERN_ERR "Device '%s' does not have a
release() function, it is broken and must be fixed. See
Documentation/core-api/kobject.rst.\n",
dev_name(dev));
- kfree(dev->driver_override.name);
kfree(p);
}
With this applied, along with the PCI driver diff from the WIP patch,
the issue is resolved. I tested this on PCI and both PoCs no longer
trigger KASAN. I also ran with other debug options enabled (lockdep,
sleep inside atomic, etc.) and hit no warnings.
I was working on a similar patch recently, but your version is better.
Not returning the string directly provides better encapsulation and
makes the API much harder to misuse.
Tested-by: Gui-Dong Han <hanguidong02@gmail.com>
Reviewed-by: Gui-Dong Han <hanguidong02@gmail.com>
> ---
> drivers/base/core.c | 2 ++
> drivers/base/dd.c | 60 +++++++++++++++++++++++++++++++
> include/linux/device.h | 81 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 143 insertions(+)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 791f9e444df8..a8cb90577d10 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -2566,6 +2566,7 @@ static void device_release(struct kobject *kobj)
> else
> WARN(1, KERN_ERR "Device '%s' does not have a release() function, it is broken and must be fixed. See Documentation/core-api/kobject.rst.\n",
> dev_name(dev));
> + kfree(dev->driver_override.name);
> kfree(p);
> }
>
> @@ -3159,6 +3160,7 @@ void device_initialize(struct device *dev)
> kobject_init(&dev->kobj, &device_ktype);
> INIT_LIST_HEAD(&dev->dma_pools);
> mutex_init(&dev->mutex);
> + spin_lock_init(&dev->driver_override.lock);
> lockdep_set_novalidate_class(&dev->mutex);
> spin_lock_init(&dev->devres_lock);
> INIT_LIST_HEAD(&dev->devres_head);
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 0354f209529c..697e36e63cab 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -381,6 +381,66 @@ static void __exit deferred_probe_exit(void)
> }
> __exitcall(deferred_probe_exit);
>
> +int __device_set_driver_override(struct device *dev, const char *s, size_t len)
> +{
> + const char *new, *old;
> + char *cp;
> +
> + if (!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 */
> + spin_lock(&dev->driver_override.lock);
> + old = dev->driver_override.name;
> + dev->driver_override.name = NULL;
> + spin_unlock(&dev->driver_override.lock);
> + kfree(old);
> +
> + return 0;
> + }
> +
> + cp = strnchr(s, len, '\n');
> + if (cp)
> + len = cp - s;
> +
> + new = kstrndup(s, len, GFP_KERNEL);
> + if (!new)
> + return -ENOMEM;
> +
> + spin_lock(&dev->driver_override.lock);
> + old = dev->driver_override.name;
> + if (cp != s) {
> + dev->driver_override.name = new;
> + spin_unlock(&dev->driver_override.lock);
> + } else {
> + /* "\n" passed - clear override */
> + dev->driver_override.name = NULL;
> + spin_unlock(&dev->driver_override.lock);
> +
> + kfree(new);
> + }
> + kfree(old);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(__device_set_driver_override);
> +
> /**
> * device_is_bound() - Check if device is bound to a driver
> * @dev: device to check
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 0be95294b6e6..4599156d5cbd 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -266,6 +266,33 @@ ssize_t device_show_string(struct device *dev, struct device_attribute *attr,
> struct dev_ext_attribute dev_attr_##_name = \
> { __ATTR(_name, (_mode) & ~0222, device_show_string, NULL), (_var) }
>
> +/**
> + * DEVICE_ATTR_DRIVER_OVERRIDE - Define sysfs driver_override attribute callbacks
> + *
> + * Generates the standard driver_override_show() and driver_override_store()
> + * sysfs callbacks and the static DEVICE_ATTR_RW(driver_override) declaration.
> + */
> +#define DEVICE_ATTR_DRIVER_OVERRIDE() \
> +static ssize_t driver_override_store(struct device *dev, \
> + struct device_attribute *attr, \
> + const char *buf, size_t count) \
> +{ \
> + int ret; \
> + \
> + ret = __device_set_driver_override(dev, buf, count); \
> + if (ret) \
> + return ret; \
> + \
> + return count; \
> +} \
> +static ssize_t driver_override_show(struct device *dev, \
> + struct device_attribute *attr, char *buf) \
> +{ \
> + guard(spinlock)(&dev->driver_override.lock); \
> + return sysfs_emit(buf, "%s\n", dev->driver_override.name); \
> +} \
> +static DEVICE_ATTR_RW(driver_override)
> +
> #define DEVICE_ATTR_IGNORE_LOCKDEP(_name, _mode, _show, _store) \
> struct device_attribute dev_attr_##_name = \
> __ATTR_IGNORE_LOCKDEP(_name, _mode, _show, _store)
> @@ -483,6 +510,8 @@ struct device_physical_location {
> * on. This shrinks the "Board Support Packages" (BSPs) and
> * minimizes board-specific #ifdefs in drivers.
> * @driver_data: Private pointer for driver specific info.
> + * @driver_override: Driver name to force a match. Do not touch directly; use
> + * device_set_driver_override() instead.
> * @links: Links to suppliers and consumers of this device.
> * @power: For device power management.
> * See Documentation/driver-api/pm/devices.rst for details.
> @@ -576,6 +605,10 @@ struct device {
> core doesn't touch it */
> void *driver_data; /* Driver data, set and get with
> dev_set_drvdata/dev_get_drvdata */
> + struct {
> + const char *name;
> + spinlock_t lock;
> + } driver_override;
> struct mutex mutex; /* mutex to synchronize calls to
> * its driver.
> */
> @@ -701,6 +734,54 @@ struct device_link {
>
> #define kobj_to_dev(__kobj) container_of_const(__kobj, struct device, kobj)
>
> +int __device_set_driver_override(struct device *dev, const char *s, size_t len);
> +
> +/**
> + * device_set_driver_override() - Helper to set or clear driver override.
> + * @dev: Device to change
> + * @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).
> + *
> + * Helper to set or clear driver override of a device.
> + *
> + * Returns: 0 on success or a negative error code on failure.
> + */
> +static inline int device_set_driver_override(struct device *dev, const char *s)
> +{
> + return __device_set_driver_override(dev, s, strlen(s));
> +}
> +
> +/**
> + * device_has_driver_override() - Check if a driver override has been set.
> + * @dev: device to check
> + *
> + * Returns true if a driver override has been set for this device.
> + */
> +static inline bool device_has_driver_override(struct device *dev)
> +{
> + guard(spinlock)(&dev->driver_override.lock);
> + return !!dev->driver_override.name;
> +}
> +
> +/**
> + * device_match_driver_override() - Match a driver against the device's driver_override.
> + * @dev: device to check
> + * @drv: driver to match against
> + *
> + * Returns > 0 if a driver override is set and matches the given driver, 0 if a
> + * driver override is set but does not match, or < 0 if a driver override is not
> + * set at all.
> + */
> +static inline int device_match_driver_override(struct device *dev,
> + const struct device_driver *drv)
> +{
> + guard(spinlock)(&dev->driver_override.lock);
> + if (dev->driver_override.name)
> + return !strcmp(dev->driver_override.name, drv->name);
> + return -1;
> +}
> +
> /**
> * device_iommu_mapped - Returns true when the device DMA is translated
> * by an IOMMU
> --
> 2.53.0
>
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 1/3] driver core: generalize driver_override in struct device
2026-03-02 8:36 ` Gui-Dong Han
@ 2026-03-02 10:05 ` Danilo Krummrich
2026-03-02 11:04 ` Gui-Dong Han
0 siblings, 1 reply; 21+ messages in thread
From: Danilo Krummrich @ 2026-03-02 10:05 UTC (permalink / raw)
To: Gui-Dong Han
Cc: gregkh, rafael, ysato, dalias, glaubitz, abelvesa, srini,
s.nawrocki, nuno.sa, driver-core, linux-kernel, imx, linux-hwmon,
linux-arm-msm, linux-sound, linux-sh, Wang Jiayue
On Mon Mar 2, 2026 at 9:36 AM CET, Gui-Dong Han wrote:
> Applying the following diff fixes the KASAN issue. It was just a minor bug.
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index a8cb90577d10..09b98f02f559 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -2556,6 +2556,7 @@ static void device_release(struct kobject *kobj)
> devres_release_all(dev);
>
> kfree(dev->dma_range_map);
> + kfree(dev->driver_override.name);
>
> if (dev->release)
> dev->release(dev);
> @@ -2566,7 +2567,6 @@ static void device_release(struct kobject *kobj)
> else
> WARN(1, KERN_ERR "Device '%s' does not have a
> release() function, it is broken and must be fixed. See
> Documentation/core-api/kobject.rst.\n",
> dev_name(dev));
> - kfree(dev->driver_override.name);
> kfree(p);
> }
Yes, we must not access dev after the release callbacks has been called; no idea
how this kfree() ended up below. Thanks for catching!
> With this applied, along with the PCI driver diff from the WIP patch,
> the issue is resolved. I tested this on PCI and both PoCs no longer
> trigger KASAN. I also ran with other debug options enabled (lockdep,
> sleep inside atomic, etc.) and hit no warnings.
>
> I was working on a similar patch recently, but your version is better.
> Not returning the string directly provides better encapsulation and
> makes the API much harder to misuse.
Ah, right, I remember you mentioned that! If you want I can add your
Co-developed-by: to this patch to account for your work.
> Tested-by: Gui-Dong Han <hanguidong02@gmail.com>
> Reviewed-by: Gui-Dong Han <hanguidong02@gmail.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] driver core: generalize driver_override in struct device
2026-03-02 10:05 ` Danilo Krummrich
@ 2026-03-02 11:04 ` Gui-Dong Han
0 siblings, 0 replies; 21+ messages in thread
From: Gui-Dong Han @ 2026-03-02 11:04 UTC (permalink / raw)
To: Danilo Krummrich
Cc: gregkh, rafael, ysato, dalias, glaubitz, abelvesa, srini,
s.nawrocki, nuno.sa, driver-core, linux-kernel, imx, linux-hwmon,
linux-arm-msm, linux-sound, linux-sh, Wang Jiayue
On Mon, Mar 2, 2026 at 6:05 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Mon Mar 2, 2026 at 9:36 AM CET, Gui-Dong Han wrote:
> > Applying the following diff fixes the KASAN issue. It was just a minor bug.
> >
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index a8cb90577d10..09b98f02f559 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -2556,6 +2556,7 @@ static void device_release(struct kobject *kobj)
> > devres_release_all(dev);
> >
> > kfree(dev->dma_range_map);
> > + kfree(dev->driver_override.name);
> >
> > if (dev->release)
> > dev->release(dev);
> > @@ -2566,7 +2567,6 @@ static void device_release(struct kobject *kobj)
> > else
> > WARN(1, KERN_ERR "Device '%s' does not have a
> > release() function, it is broken and must be fixed. See
> > Documentation/core-api/kobject.rst.\n",
> > dev_name(dev));
> > - kfree(dev->driver_override.name);
> > kfree(p);
> > }
>
> Yes, we must not access dev after the release callbacks has been called; no idea
> how this kfree() ended up below. Thanks for catching!
>
> > With this applied, along with the PCI driver diff from the WIP patch,
> > the issue is resolved. I tested this on PCI and both PoCs no longer
> > trigger KASAN. I also ran with other debug options enabled (lockdep,
> > sleep inside atomic, etc.) and hit no warnings.
> >
> > I was working on a similar patch recently, but your version is better.
> > Not returning the string directly provides better encapsulation and
> > makes the API much harder to misuse.
>
> Ah, right, I remember you mentioned that! If you want I can add your
> Co-developed-by: to this patch to account for your work.
That would be great, yes please. Thank you!
And thanks for all your hard work on this patch series, it's a really
solid improvement.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] driver core: generalize driver_override in struct device
2026-03-02 0:25 ` [PATCH 1/3] driver core: generalize driver_override in struct device Danilo Krummrich
2026-03-02 7:35 ` Gui-Dong Han
2026-03-02 8:36 ` Gui-Dong Han
@ 2026-03-02 10:00 ` Geert Uytterhoeven
2026-03-02 10:26 ` Danilo Krummrich
2026-03-02 10:23 ` Armin Wolf
3 siblings, 1 reply; 21+ messages in thread
From: Geert Uytterhoeven @ 2026-03-02 10:00 UTC (permalink / raw)
To: Danilo Krummrich
Cc: gregkh, rafael, hanguidong02, ysato, dalias, glaubitz, abelvesa,
srini, s.nawrocki, nuno.sa, driver-core, linux-kernel, imx,
linux-hwmon, linux-arm-msm, linux-sound, linux-sh
Hi Danilo,
On Mon, 2 Mar 2026 at 01:28, Danilo Krummrich <dakr@kernel.org> wrote:
> Currently, there are 12 busses (including platform and PCI) that
> duplicate the driver_override logic for their individual devices.
>
> All of them seem to be prone to the bug described in [1].
>
> While this could be solved for every bus individually using a separate
> lock, solving this in the driver-core generically results in less (and
> cleaner) changes overall.
>
> Thus, move driver_override to struct device, provide corresponding
> accessors for busses and handle locking with a separate lock internally.
>
> In particular, add device_set_driver_override(),
> device_has_driver_override(), device_match_driver_override() and a
> helper, DEVICE_ATTR_DRIVER_OVERRIDE(), to declare the corresponding
> sysfs store() and show() callbacks.
>
> Until all busses have migrated, keep driver_set_override() in place.
>
> Note that we can't use the device lock for the reasons described in [2].
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=220789 [1]
> Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/ [2]
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
Thanks for your patch!
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -381,6 +381,66 @@ static void __exit deferred_probe_exit(void)
> }
> __exitcall(deferred_probe_exit);
>
> +int __device_set_driver_override(struct device *dev, const char *s, size_t len)
> +{
> + const char *new, *old;
> + char *cp;
> +
> + if (!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);
> +
The newline case below is is basically the same case as the empty
string. Hence if you would move the newline check here:
if (len) {
cp = strnchr(s, len, '\n');
if (cp)
len = cp - s;
}
then the "cp != s" check below is no longer needed.
> + if (!len) {
> + /* Empty string passed - clear override */
> + spin_lock(&dev->driver_override.lock);
> + old = dev->driver_override.name;
> + dev->driver_override.name = NULL;
> + spin_unlock(&dev->driver_override.lock);
> + kfree(old);
> +
> + return 0;
> + }
Also, this block can be eliminated completely...
> +
> + cp = strnchr(s, len, '\n');
> + if (cp)
> + len = cp - s;
> +
> + new = kstrndup(s, len, GFP_KERNEL);
> + if (!new)
> + return -ENOMEM;
... by pre-initializing new to NULL, and making the allocation of new
conditional on len being non-zero.
> +
> + spin_lock(&dev->driver_override.lock);
> + old = dev->driver_override.name;
> + if (cp != s) {
> + dev->driver_override.name = new;
> + spin_unlock(&dev->driver_override.lock);
> + } else {
> + /* "\n" passed - clear override */
> + dev->driver_override.name = NULL;
> + spin_unlock(&dev->driver_override.lock);
> +
> + kfree(new);
> + }
> + kfree(old);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(__device_set_driver_override);
> +
> /**
> * device_is_bound() - Check if device is bound to a driver
> * @dev: device to check
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/3] driver core: generalize driver_override in struct device
2026-03-02 10:00 ` Geert Uytterhoeven
@ 2026-03-02 10:26 ` Danilo Krummrich
2026-03-02 10:38 ` Geert Uytterhoeven
0 siblings, 1 reply; 21+ messages in thread
From: Danilo Krummrich @ 2026-03-02 10:26 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: gregkh, rafael, hanguidong02, ysato, dalias, glaubitz, abelvesa,
srini, s.nawrocki, nuno.sa, driver-core, linux-kernel, imx,
linux-hwmon, linux-arm-msm, linux-sound, linux-sh
On Mon Mar 2, 2026 at 11:00 AM CET, Geert Uytterhoeven wrote:
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -381,6 +381,66 @@ static void __exit deferred_probe_exit(void)
>> }
>> __exitcall(deferred_probe_exit);
>>
>> +int __device_set_driver_override(struct device *dev, const char *s, size_t len)
>> +{
>> + const char *new, *old;
>> + char *cp;
>> +
>> + if (!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);
>> +
>
> The newline case below is is basically the same case as the empty
> string. Hence if you would move the newline check here:
>
> if (len) {
> cp = strnchr(s, len, '\n');
> if (cp)
> len = cp - s;
> }
>
> then the "cp != s" check below is no longer needed.
>
>> + if (!len) {
>> + /* Empty string passed - clear override */
>> + spin_lock(&dev->driver_override.lock);
>> + old = dev->driver_override.name;
>> + dev->driver_override.name = NULL;
>> + spin_unlock(&dev->driver_override.lock);
>> + kfree(old);
>> +
>> + return 0;
>> + }
>
> Also, this block can be eliminated completely...
>
>> +
>> + cp = strnchr(s, len, '\n');
>> + if (cp)
>> + len = cp - s;
>> +
>> + new = kstrndup(s, len, GFP_KERNEL);
>> + if (!new)
>> + return -ENOMEM;
>
> ... by pre-initializing new to NULL, and making the allocation of new
> conditional on len being non-zero.
>
>> +
>> + spin_lock(&dev->driver_override.lock);
>> + old = dev->driver_override.name;
>> + if (cp != s) {
>> + dev->driver_override.name = new;
>> + spin_unlock(&dev->driver_override.lock);
>> + } else {
>> + /* "\n" passed - clear override */
>> + dev->driver_override.name = NULL;
>> + spin_unlock(&dev->driver_override.lock);
>> +
>> + kfree(new);
>> + }
>> + kfree(old);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(__device_set_driver_override);
This is essentially a copy of driver_set_override(). Except for the required
minor changes I intentionally kept it "as is" as it will go through -fixes and
we know it works properly.
Do you mind sending a follow-up patch with your suggested improvements?
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/3] driver core: generalize driver_override in struct device
2026-03-02 10:26 ` Danilo Krummrich
@ 2026-03-02 10:38 ` Geert Uytterhoeven
2026-03-02 11:03 ` Danilo Krummrich
0 siblings, 1 reply; 21+ messages in thread
From: Geert Uytterhoeven @ 2026-03-02 10:38 UTC (permalink / raw)
To: Danilo Krummrich
Cc: gregkh, rafael, hanguidong02, ysato, dalias, glaubitz, abelvesa,
srini, s.nawrocki, nuno.sa, driver-core, linux-kernel, imx,
linux-hwmon, linux-arm-msm, linux-sound, linux-sh
Hi Danilo,
On Mon, 2 Mar 2026 at 11:26, Danilo Krummrich <dakr@kernel.org> wrote:
> On Mon Mar 2, 2026 at 11:00 AM CET, Geert Uytterhoeven wrote:
> >> --- a/drivers/base/dd.c
> >> +++ b/drivers/base/dd.c
> >> @@ -381,6 +381,66 @@ static void __exit deferred_probe_exit(void)
> >> }
> >> __exitcall(deferred_probe_exit);
> >>
> >> +int __device_set_driver_override(struct device *dev, const char *s, size_t len)
> >> +{
> >> + const char *new, *old;
> >> + char *cp;
> >> +
> >> + if (!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);
> >> +
> >
> > The newline case below is is basically the same case as the empty
> > string. Hence if you would move the newline check here:
> >
> > if (len) {
> > cp = strnchr(s, len, '\n');
> > if (cp)
> > len = cp - s;
> > }
> >
> > then the "cp != s" check below is no longer needed.
> >
> >> + if (!len) {
> >> + /* Empty string passed - clear override */
> >> + spin_lock(&dev->driver_override.lock);
> >> + old = dev->driver_override.name;
> >> + dev->driver_override.name = NULL;
> >> + spin_unlock(&dev->driver_override.lock);
> >> + kfree(old);
> >> +
> >> + return 0;
> >> + }
> >
> > Also, this block can be eliminated completely...
> >
> >> +
> >> + cp = strnchr(s, len, '\n');
> >> + if (cp)
> >> + len = cp - s;
> >> +
> >> + new = kstrndup(s, len, GFP_KERNEL);
> >> + if (!new)
> >> + return -ENOMEM;
> >
> > ... by pre-initializing new to NULL, and making the allocation of new
> > conditional on len being non-zero.
> >
> >> +
> >> + spin_lock(&dev->driver_override.lock);
> >> + old = dev->driver_override.name;
> >> + if (cp != s) {
> >> + dev->driver_override.name = new;
> >> + spin_unlock(&dev->driver_override.lock);
> >> + } else {
> >> + /* "\n" passed - clear override */
> >> + dev->driver_override.name = NULL;
> >> + spin_unlock(&dev->driver_override.lock);
> >> +
> >> + kfree(new);
> >> + }
> >> + kfree(old);
> >> +
> >> + return 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(__device_set_driver_override);
>
> This is essentially a copy of driver_set_override(). Except for the required
> minor changes I intentionally kept it "as is" as it will go through -fixes and
> we know it works properly.
So I will have two to fix? ;-)
> Do you mind sending a follow-up patch with your suggested improvements?
Adding it to my TODO list, if this patch makes it as-is.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/3] driver core: generalize driver_override in struct device
2026-03-02 10:38 ` Geert Uytterhoeven
@ 2026-03-02 11:03 ` Danilo Krummrich
0 siblings, 0 replies; 21+ messages in thread
From: Danilo Krummrich @ 2026-03-02 11:03 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: gregkh, rafael, hanguidong02, ysato, dalias, glaubitz, abelvesa,
srini, s.nawrocki, nuno.sa, driver-core, linux-kernel, imx,
linux-hwmon, linux-arm-msm, linux-sound, linux-sh
On Mon Mar 2, 2026 at 11:38 AM CET, Geert Uytterhoeven wrote:
> So I will have two to fix? ;-)
No, once all busses have been migrated driver_set_override() will be removed. :)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] driver core: generalize driver_override in struct device
2026-03-02 0:25 ` [PATCH 1/3] driver core: generalize driver_override in struct device Danilo Krummrich
` (2 preceding siblings ...)
2026-03-02 10:00 ` Geert Uytterhoeven
@ 2026-03-02 10:23 ` Armin Wolf
2026-03-02 16:28 ` Danilo Krummrich
3 siblings, 1 reply; 21+ messages in thread
From: Armin Wolf @ 2026-03-02 10:23 UTC (permalink / raw)
To: Danilo Krummrich, gregkh, rafael, hanguidong02, ysato, dalias,
glaubitz, abelvesa, srini, s.nawrocki, nuno.sa
Cc: driver-core, linux-kernel, imx, linux-hwmon, linux-arm-msm,
linux-sound, linux-sh
Am 02.03.26 um 01:25 schrieb Danilo Krummrich:
> Currently, there are 12 busses (including platform and PCI) that
> duplicate the driver_override logic for their individual devices.
>
> All of them seem to be prone to the bug described in [1].
>
> While this could be solved for every bus individually using a separate
> lock, solving this in the driver-core generically results in less (and
> cleaner) changes overall.
>
> Thus, move driver_override to struct device, provide corresponding
> accessors for busses and handle locking with a separate lock internally.
>
> In particular, add device_set_driver_override(),
> device_has_driver_override(), device_match_driver_override() and a
> helper, DEVICE_ATTR_DRIVER_OVERRIDE(), to declare the corresponding
> sysfs store() and show() callbacks.
Nice patch series, centralizing the handling of driver_override in the
device core removes a lot of duplicated code.
I wonder if we can also move DEVICE_ATTR_DRIVER_OVERRIDE() into the device
core by registering the associated sysfs attribute inside bus_add_device().
Bus types that honor driver_override could for example set a boolean flag
inside struct bus_type to tell the device core that driver_override is
supported.
However this is just a suggestion, the current patch series seems fine to me.
Thanks,
Armin Wolf
> Until all busses have migrated, keep driver_set_override() in place.
>
> Note that we can't use the device lock for the reasons described in [2].
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=220789 [1]
> Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/ [2]
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
> drivers/base/core.c | 2 ++
> drivers/base/dd.c | 60 +++++++++++++++++++++++++++++++
> include/linux/device.h | 81 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 143 insertions(+)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 791f9e444df8..a8cb90577d10 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -2566,6 +2566,7 @@ static void device_release(struct kobject *kobj)
> else
> WARN(1, KERN_ERR "Device '%s' does not have a release() function, it is broken and must be fixed. See Documentation/core-api/kobject.rst.\n",
> dev_name(dev));
> + kfree(dev->driver_override.name);
> kfree(p);
> }
>
> @@ -3159,6 +3160,7 @@ void device_initialize(struct device *dev)
> kobject_init(&dev->kobj, &device_ktype);
> INIT_LIST_HEAD(&dev->dma_pools);
> mutex_init(&dev->mutex);
> + spin_lock_init(&dev->driver_override.lock);
> lockdep_set_novalidate_class(&dev->mutex);
> spin_lock_init(&dev->devres_lock);
> INIT_LIST_HEAD(&dev->devres_head);
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 0354f209529c..697e36e63cab 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -381,6 +381,66 @@ static void __exit deferred_probe_exit(void)
> }
> __exitcall(deferred_probe_exit);
>
> +int __device_set_driver_override(struct device *dev, const char *s, size_t len)
> +{
> + const char *new, *old;
> + char *cp;
> +
> + if (!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 */
> + spin_lock(&dev->driver_override.lock);
> + old = dev->driver_override.name;
> + dev->driver_override.name = NULL;
> + spin_unlock(&dev->driver_override.lock);
> + kfree(old);
> +
> + return 0;
> + }
> +
> + cp = strnchr(s, len, '\n');
> + if (cp)
> + len = cp - s;
> +
> + new = kstrndup(s, len, GFP_KERNEL);
> + if (!new)
> + return -ENOMEM;
> +
> + spin_lock(&dev->driver_override.lock);
> + old = dev->driver_override.name;
> + if (cp != s) {
> + dev->driver_override.name = new;
> + spin_unlock(&dev->driver_override.lock);
> + } else {
> + /* "\n" passed - clear override */
> + dev->driver_override.name = NULL;
> + spin_unlock(&dev->driver_override.lock);
> +
> + kfree(new);
> + }
> + kfree(old);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(__device_set_driver_override);
> +
> /**
> * device_is_bound() - Check if device is bound to a driver
> * @dev: device to check
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 0be95294b6e6..4599156d5cbd 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -266,6 +266,33 @@ ssize_t device_show_string(struct device *dev, struct device_attribute *attr,
> struct dev_ext_attribute dev_attr_##_name = \
> { __ATTR(_name, (_mode) & ~0222, device_show_string, NULL), (_var) }
>
> +/**
> + * DEVICE_ATTR_DRIVER_OVERRIDE - Define sysfs driver_override attribute callbacks
> + *
> + * Generates the standard driver_override_show() and driver_override_store()
> + * sysfs callbacks and the static DEVICE_ATTR_RW(driver_override) declaration.
> + */
> +#define DEVICE_ATTR_DRIVER_OVERRIDE() \
> +static ssize_t driver_override_store(struct device *dev, \
> + struct device_attribute *attr, \
> + const char *buf, size_t count) \
> +{ \
> + int ret; \
> + \
> + ret = __device_set_driver_override(dev, buf, count); \
> + if (ret) \
> + return ret; \
> + \
> + return count; \
> +} \
> +static ssize_t driver_override_show(struct device *dev, \
> + struct device_attribute *attr, char *buf) \
> +{ \
> + guard(spinlock)(&dev->driver_override.lock); \
> + return sysfs_emit(buf, "%s\n", dev->driver_override.name); \
> +} \
> +static DEVICE_ATTR_RW(driver_override)
> +
> #define DEVICE_ATTR_IGNORE_LOCKDEP(_name, _mode, _show, _store) \
> struct device_attribute dev_attr_##_name = \
> __ATTR_IGNORE_LOCKDEP(_name, _mode, _show, _store)
> @@ -483,6 +510,8 @@ struct device_physical_location {
> * on. This shrinks the "Board Support Packages" (BSPs) and
> * minimizes board-specific #ifdefs in drivers.
> * @driver_data: Private pointer for driver specific info.
> + * @driver_override: Driver name to force a match. Do not touch directly; use
> + * device_set_driver_override() instead.
> * @links: Links to suppliers and consumers of this device.
> * @power: For device power management.
> * See Documentation/driver-api/pm/devices.rst for details.
> @@ -576,6 +605,10 @@ struct device {
> core doesn't touch it */
> void *driver_data; /* Driver data, set and get with
> dev_set_drvdata/dev_get_drvdata */
> + struct {
> + const char *name;
> + spinlock_t lock;
> + } driver_override;
> struct mutex mutex; /* mutex to synchronize calls to
> * its driver.
> */
> @@ -701,6 +734,54 @@ struct device_link {
>
> #define kobj_to_dev(__kobj) container_of_const(__kobj, struct device, kobj)
>
> +int __device_set_driver_override(struct device *dev, const char *s, size_t len);
> +
> +/**
> + * device_set_driver_override() - Helper to set or clear driver override.
> + * @dev: Device to change
> + * @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).
> + *
> + * Helper to set or clear driver override of a device.
> + *
> + * Returns: 0 on success or a negative error code on failure.
> + */
> +static inline int device_set_driver_override(struct device *dev, const char *s)
> +{
> + return __device_set_driver_override(dev, s, strlen(s));
> +}
> +
> +/**
> + * device_has_driver_override() - Check if a driver override has been set.
> + * @dev: device to check
> + *
> + * Returns true if a driver override has been set for this device.
> + */
> +static inline bool device_has_driver_override(struct device *dev)
> +{
> + guard(spinlock)(&dev->driver_override.lock);
> + return !!dev->driver_override.name;
> +}
> +
> +/**
> + * device_match_driver_override() - Match a driver against the device's driver_override.
> + * @dev: device to check
> + * @drv: driver to match against
> + *
> + * Returns > 0 if a driver override is set and matches the given driver, 0 if a
> + * driver override is set but does not match, or < 0 if a driver override is not
> + * set at all.
> + */
> +static inline int device_match_driver_override(struct device *dev,
> + const struct device_driver *drv)
> +{
> + guard(spinlock)(&dev->driver_override.lock);
> + if (dev->driver_override.name)
> + return !strcmp(dev->driver_override.name, drv->name);
> + return -1;
> +}
> +
> /**
> * device_iommu_mapped - Returns true when the device DMA is translated
> * by an IOMMU
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/3] driver core: generalize driver_override in struct device
2026-03-02 10:23 ` Armin Wolf
@ 2026-03-02 16:28 ` Danilo Krummrich
0 siblings, 0 replies; 21+ messages in thread
From: Danilo Krummrich @ 2026-03-02 16:28 UTC (permalink / raw)
To: Armin Wolf
Cc: gregkh, rafael, hanguidong02, ysato, dalias, glaubitz, abelvesa,
srini, s.nawrocki, nuno.sa, driver-core, linux-kernel, imx,
linux-hwmon, linux-arm-msm, linux-sound, linux-sh
On Mon Mar 2, 2026 at 11:23 AM CET, Armin Wolf wrote:
> Nice patch series, centralizing the handling of driver_override in the
> device core removes a lot of duplicated code.
>
> I wonder if we can also move DEVICE_ATTR_DRIVER_OVERRIDE() into the device
> core by registering the associated sysfs attribute inside bus_add_device().
>
> Bus types that honor driver_override could for example set a boolean flag
> inside struct bus_type to tell the device core that driver_override is
> supported.
>
> However this is just a suggestion, the current patch series seems fine to me.
That's a very good suggestion, thanks for pointing it out. In fact, I already
thought of this at some point, but then forgot to implement it. :)
^ permalink raw reply [flat|nested] 21+ messages in thread