public inbox for linux-sh@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] driver core: generalize driver_override infrastructure
@ 2026-03-02  0:25 Danilo Krummrich
  2026-03-02  0:25 ` [PATCH 1/3] driver core: generalize driver_override in struct device Danilo Krummrich
                   ` (3 more replies)
  0 siblings, 4 replies; 21+ messages in thread
From: Danilo Krummrich @ 2026-03-02  0:25 UTC (permalink / raw)
  To: 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, 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.

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].

This patch series includes the migration of the platform bus; patches for all
other affected busses still need to be extracted as a follow-up of the WIP
treewide patch in [3].

[1] https://bugzilla.kernel.org/show_bug.cgi?id=220789
[2] https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/
[3] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=driver_override

Danilo Krummrich (3):
  driver core: generalize driver_override in struct device
  hwmon: axi-fan: don't use driver_override as IRQ name
  driver core: platform: use generic driver_override infrastructure

 arch/sh/drivers/platform_early.c |  6 ++-
 drivers/base/core.c              |  2 +
 drivers/base/dd.c                | 60 +++++++++++++++++++++++
 drivers/base/platform.c          | 35 ++------------
 drivers/bus/simple-pm-bus.c      |  4 +-
 drivers/clk/imx/clk-scu.c        |  3 +-
 drivers/hwmon/axi-fan-control.c  |  2 +-
 drivers/slimbus/qcom-ngd-ctrl.c  |  6 +--
 include/linux/device.h           | 81 ++++++++++++++++++++++++++++++++
 include/linux/platform_device.h  |  5 --
 sound/soc/samsung/i2s.c          |  6 +--
 11 files changed, 161 insertions(+), 49 deletions(-)


base-commit: 78437ab3b769f80526416570f60173c89858dd84
-- 
2.53.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/3] driver core: generalize driver_override in struct device
  2026-03-02  0:25 [PATCH 0/3] driver core: generalize driver_override infrastructure Danilo Krummrich
@ 2026-03-02  0:25 ` Danilo Krummrich
  2026-03-02  7:35   ` Gui-Dong Han
                     ` (3 more replies)
  2026-03-02  0:25 ` [PATCH 2/3] hwmon: axi-fan: don't use driver_override as IRQ name Danilo Krummrich
                   ` (2 subsequent siblings)
  3 siblings, 4 replies; 21+ messages in thread
From: Danilo Krummrich @ 2026-03-02  0:25 UTC (permalink / raw)
  To: 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, 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.

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
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 2/3] hwmon: axi-fan: don't use driver_override as IRQ name
  2026-03-02  0:25 [PATCH 0/3] driver core: generalize driver_override infrastructure Danilo Krummrich
  2026-03-02  0:25 ` [PATCH 1/3] driver core: generalize driver_override in struct device Danilo Krummrich
@ 2026-03-02  0:25 ` Danilo Krummrich
  2026-03-02  0:51   ` Guenter Roeck
  2026-03-02  0:25 ` [PATCH 3/3] driver core: platform: use generic driver_override infrastructure Danilo Krummrich
  2026-03-02  9:41 ` [PATCH 0/3] driver core: generalize " Gui-Dong Han
  3 siblings, 1 reply; 21+ messages in thread
From: Danilo Krummrich @ 2026-03-02  0:25 UTC (permalink / raw)
  To: 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, Danilo Krummrich

Do not use driver_override as IRQ name, as it is not guaranteed to point
to a valid string; use dev_name() instead.

Fixes: 8412b410fa5e ("hwmon: Support ADI Fan Control IP")
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/hwmon/axi-fan-control.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/axi-fan-control.c b/drivers/hwmon/axi-fan-control.c
index b7bb325c3ad9..ec4bbb104449 100644
--- a/drivers/hwmon/axi-fan-control.c
+++ b/drivers/hwmon/axi-fan-control.c
@@ -507,7 +507,7 @@ static int axi_fan_control_probe(struct platform_device *pdev)
 	ret = devm_request_threaded_irq(&pdev->dev, ctl->irq, NULL,
 					axi_fan_control_irq_handler,
 					IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
-					pdev->driver_override, ctl);
+					dev_name(&pdev->dev), ctl);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret,
 				     "failed to request an irq\n");
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 3/3] driver core: platform: use generic driver_override infrastructure
  2026-03-02  0:25 [PATCH 0/3] driver core: generalize driver_override infrastructure Danilo Krummrich
  2026-03-02  0:25 ` [PATCH 1/3] driver core: generalize driver_override in struct device Danilo Krummrich
  2026-03-02  0:25 ` [PATCH 2/3] hwmon: axi-fan: don't use driver_override as IRQ name Danilo Krummrich
@ 2026-03-02  0:25 ` Danilo Krummrich
  2026-03-02  8:55   ` Gui-Dong Han
  2026-03-02  9:41 ` [PATCH 0/3] driver core: generalize " Gui-Dong Han
  3 siblings, 1 reply; 21+ messages in thread
From: Danilo Krummrich @ 2026-03-02  0:25 UTC (permalink / raw)
  To: 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, Danilo Krummrich

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]

Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/ [1]
Reported-by: Gui-Dong Han <hanguidong02@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220789
Fixes: 3d713e0e382e ("driver core: platform: add device binding path 'driver_override'")
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 arch/sh/drivers/platform_early.c |  6 ++++--
 drivers/base/platform.c          | 35 +++++---------------------------
 drivers/bus/simple-pm-bus.c      |  4 ++--
 drivers/clk/imx/clk-scu.c        |  3 +--
 drivers/slimbus/qcom-ngd-ctrl.c  |  6 ++----
 include/linux/platform_device.h  |  5 -----
 sound/soc/samsung/i2s.c          |  6 +++---
 7 files changed, 17 insertions(+), 48 deletions(-)

diff --git a/arch/sh/drivers/platform_early.c b/arch/sh/drivers/platform_early.c
index 143747c45206..3cd17bb0be67 100644
--- a/arch/sh/drivers/platform_early.c
+++ b/arch/sh/drivers/platform_early.c
@@ -25,10 +25,12 @@ static int platform_match(struct device *dev, struct device_driver *drv)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct platform_driver *pdrv = to_platform_driver(drv);
+	int ret;
 
 	/* When driver_override is set, only bind to the matching driver */
-	if (pdev->driver_override)
-		return !strcmp(pdev->driver_override, drv->name);
+	ret = device_match_driver_override(dev, drv);
+	if (ret >= 0)
+		return ret;
 
 	/* Then try to match against the id table */
 	if (pdrv->id_table)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index b45d41b018ca..22ae87921a7a 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -603,7 +603,6 @@ static void platform_device_release(struct device *dev)
 	kfree(pa->pdev.dev.platform_data);
 	kfree(pa->pdev.mfd_cell);
 	kfree(pa->pdev.resource);
-	kfree(pa->pdev.driver_override);
 	kfree(pa);
 }
 
@@ -1306,33 +1305,7 @@ static ssize_t numa_node_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(numa_node);
 
-static ssize_t driver_override_show(struct device *dev,
-				    struct device_attribute *attr, char *buf)
-{
-	struct platform_device *pdev = to_platform_device(dev);
-	ssize_t len;
-
-	device_lock(dev);
-	len = sysfs_emit(buf, "%s\n", pdev->driver_override);
-	device_unlock(dev);
-
-	return len;
-}
-
-static ssize_t driver_override_store(struct device *dev,
-				     struct device_attribute *attr,
-				     const char *buf, size_t count)
-{
-	struct platform_device *pdev = to_platform_device(dev);
-	int ret;
-
-	ret = driver_set_override(dev, &pdev->driver_override, buf, count);
-	if (ret)
-		return ret;
-
-	return count;
-}
-static DEVICE_ATTR_RW(driver_override);
+DEVICE_ATTR_DRIVER_OVERRIDE();
 
 static struct attribute *platform_dev_attrs[] = {
 	&dev_attr_modalias.attr,
@@ -1377,10 +1350,12 @@ static int platform_match(struct device *dev, const struct device_driver *drv)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct platform_driver *pdrv = to_platform_driver(drv);
+	int ret;
 
 	/* When driver_override is set, only bind to the matching driver */
-	if (pdev->driver_override)
-		return !strcmp(pdev->driver_override, drv->name);
+	ret = device_match_driver_override(dev, drv);
+	if (ret >= 0)
+		return ret;
 
 	/* Attempt an OF style match first */
 	if (of_driver_match_device(dev, drv))
diff --git a/drivers/bus/simple-pm-bus.c b/drivers/bus/simple-pm-bus.c
index 3f00d953fb9a..c920bd6fbaaf 100644
--- a/drivers/bus/simple-pm-bus.c
+++ b/drivers/bus/simple-pm-bus.c
@@ -36,7 +36,7 @@ static int simple_pm_bus_probe(struct platform_device *pdev)
 	 * that's not listed in simple_pm_bus_of_match. We don't want to do any
 	 * of the simple-pm-bus tasks for these devices, so return early.
 	 */
-	if (pdev->driver_override)
+	if (device_has_driver_override(&pdev->dev))
 		return 0;
 
 	match = of_match_device(dev->driver->of_match_table, dev);
@@ -78,7 +78,7 @@ static void simple_pm_bus_remove(struct platform_device *pdev)
 {
 	const void *data = of_device_get_match_data(&pdev->dev);
 
-	if (pdev->driver_override || data)
+	if (device_has_driver_override(&pdev->dev) || data)
 		return;
 
 	dev_dbg(&pdev->dev, "%s\n", __func__);
diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
index a85ec48a798b..9b33df9967ec 100644
--- a/drivers/clk/imx/clk-scu.c
+++ b/drivers/clk/imx/clk-scu.c
@@ -706,8 +706,7 @@ struct clk_hw *imx_clk_scu_alloc_dev(const char *name,
 	if (ret)
 		goto put_device;
 
-	ret = driver_set_override(&pdev->dev, &pdev->driver_override,
-				  "imx-scu-clk", strlen("imx-scu-clk"));
+	ret = device_set_driver_override(&pdev->dev, "imx-scu-clk");
 	if (ret)
 		goto put_device;
 
diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
index 9aa7218b4e8d..1ed6be6e85d2 100644
--- a/drivers/slimbus/qcom-ngd-ctrl.c
+++ b/drivers/slimbus/qcom-ngd-ctrl.c
@@ -1535,10 +1535,8 @@ static int of_qcom_slim_ngd_register(struct device *parent,
 		ngd->id = id;
 		ngd->pdev->dev.parent = parent;
 
-		ret = driver_set_override(&ngd->pdev->dev,
-					  &ngd->pdev->driver_override,
-					  QCOM_SLIM_NGD_DRV_NAME,
-					  strlen(QCOM_SLIM_NGD_DRV_NAME));
+		ret = device_set_driver_override(&ngd->pdev->dev,
+						 QCOM_SLIM_NGD_DRV_NAME);
 		if (ret) {
 			platform_device_put(ngd->pdev);
 			kfree(ngd);
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 813da101b5bf..ed1d50d1c3c1 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -31,11 +31,6 @@ struct platform_device {
 	struct resource	*resource;
 
 	const struct platform_device_id	*id_entry;
-	/*
-	 * Driver name to force a match.  Do not set directly, because core
-	 * frees it.  Use driver_set_override() to set or clear it.
-	 */
-	const char *driver_override;
 
 	/* MFD cell pointer */
 	struct mfd_cell *mfd_cell;
diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index e9964f0e010a..140907a41a70 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -1360,10 +1360,10 @@ static int i2s_create_secondary_device(struct samsung_i2s_priv *priv)
 	if (!pdev_sec)
 		return -ENOMEM;
 
-	pdev_sec->driver_override = kstrdup("samsung-i2s", GFP_KERNEL);
-	if (!pdev_sec->driver_override) {
+	ret = device_set_driver_override(&pdev_sec->dev, "samsung-i2s");
+	if (ret) {
 		platform_device_put(pdev_sec);
-		return -ENOMEM;
+		return ret;
 	}
 
 	ret = platform_device_add(pdev_sec);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/3] hwmon: axi-fan: don't use driver_override as IRQ name
  2026-03-02  0:25 ` [PATCH 2/3] hwmon: axi-fan: don't use driver_override as IRQ name Danilo Krummrich
@ 2026-03-02  0:51   ` Guenter Roeck
  2026-03-02 10:00     ` Danilo Krummrich
  2026-03-02 11:02     ` Nuno Sá
  0 siblings, 2 replies; 21+ messages in thread
From: Guenter Roeck @ 2026-03-02  0:51 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

On 3/1/26 16:25, Danilo Krummrich wrote:
> Do not use driver_override as IRQ name, as it is not guaranteed to point
> to a valid string; use dev_name() instead.
> 
> Fixes: 8412b410fa5e ("hwmon: Support ADI Fan Control IP")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>   drivers/hwmon/axi-fan-control.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/axi-fan-control.c b/drivers/hwmon/axi-fan-control.c
> index b7bb325c3ad9..ec4bbb104449 100644
> --- a/drivers/hwmon/axi-fan-control.c
> +++ b/drivers/hwmon/axi-fan-control.c
> @@ -507,7 +507,7 @@ static int axi_fan_control_probe(struct platform_device *pdev)
>   	ret = devm_request_threaded_irq(&pdev->dev, ctl->irq, NULL,
>   					axi_fan_control_irq_handler,
>   					IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
> -					pdev->driver_override, ctl);
> +					dev_name(&pdev->dev), ctl);

The devm_request_threaded_irq() API documentation says:

@devname:    An ascii name for the claiming device, dev_name(dev) if NULL

So NULL should be sufficient.

Nuno, was there a special reason to use driver_override ?

Thanks,
Guenter


^ 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
                     ` (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 3/3] driver core: platform: use generic driver_override infrastructure
  2026-03-02  0:25 ` [PATCH 3/3] driver core: platform: use generic driver_override infrastructure Danilo Krummrich
@ 2026-03-02  8:55   ` Gui-Dong Han
  0 siblings, 0 replies; 21+ messages in thread
From: Gui-Dong Han @ 2026-03-02  8:55 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:28 AM Danilo Krummrich <dakr@kernel.org> wrote:
>
> 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]
>
> Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/ [1]
> Reported-by: Gui-Dong Han <hanguidong02@gmail.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220789
> Fixes: 3d713e0e382e ("driver core: platform: add device binding path 'driver_override'")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Looks good to me.

One minor note for the commit message: this UAF can also be triggered
from the bind_store path, not just __driver_attach(). It would be good
to mention this as well.

Reviewed-by: Gui-Dong Han <hanguidong02@gmail.com>

> ---
>  arch/sh/drivers/platform_early.c |  6 ++++--
>  drivers/base/platform.c          | 35 +++++---------------------------
>  drivers/bus/simple-pm-bus.c      |  4 ++--
>  drivers/clk/imx/clk-scu.c        |  3 +--
>  drivers/slimbus/qcom-ngd-ctrl.c  |  6 ++----
>  include/linux/platform_device.h  |  5 -----
>  sound/soc/samsung/i2s.c          |  6 +++---
>  7 files changed, 17 insertions(+), 48 deletions(-)
>
> diff --git a/arch/sh/drivers/platform_early.c b/arch/sh/drivers/platform_early.c
> index 143747c45206..3cd17bb0be67 100644
> --- a/arch/sh/drivers/platform_early.c
> +++ b/arch/sh/drivers/platform_early.c
> @@ -25,10 +25,12 @@ static int platform_match(struct device *dev, struct device_driver *drv)
>  {
>         struct platform_device *pdev = to_platform_device(dev);
>         struct platform_driver *pdrv = to_platform_driver(drv);
> +       int ret;
>
>         /* When driver_override is set, only bind to the matching driver */
> -       if (pdev->driver_override)
> -               return !strcmp(pdev->driver_override, drv->name);
> +       ret = device_match_driver_override(dev, drv);
> +       if (ret >= 0)
> +               return ret;
>
>         /* Then try to match against the id table */
>         if (pdrv->id_table)
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index b45d41b018ca..22ae87921a7a 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -603,7 +603,6 @@ static void platform_device_release(struct device *dev)
>         kfree(pa->pdev.dev.platform_data);
>         kfree(pa->pdev.mfd_cell);
>         kfree(pa->pdev.resource);
> -       kfree(pa->pdev.driver_override);
>         kfree(pa);
>  }
>
> @@ -1306,33 +1305,7 @@ static ssize_t numa_node_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(numa_node);
>
> -static ssize_t driver_override_show(struct device *dev,
> -                                   struct device_attribute *attr, char *buf)
> -{
> -       struct platform_device *pdev = to_platform_device(dev);
> -       ssize_t len;
> -
> -       device_lock(dev);
> -       len = sysfs_emit(buf, "%s\n", pdev->driver_override);
> -       device_unlock(dev);
> -
> -       return len;
> -}
> -
> -static ssize_t driver_override_store(struct device *dev,
> -                                    struct device_attribute *attr,
> -                                    const char *buf, size_t count)
> -{
> -       struct platform_device *pdev = to_platform_device(dev);
> -       int ret;
> -
> -       ret = driver_set_override(dev, &pdev->driver_override, buf, count);
> -       if (ret)
> -               return ret;
> -
> -       return count;
> -}
> -static DEVICE_ATTR_RW(driver_override);
> +DEVICE_ATTR_DRIVER_OVERRIDE();
>
>  static struct attribute *platform_dev_attrs[] = {
>         &dev_attr_modalias.attr,
> @@ -1377,10 +1350,12 @@ static int platform_match(struct device *dev, const struct device_driver *drv)
>  {
>         struct platform_device *pdev = to_platform_device(dev);
>         struct platform_driver *pdrv = to_platform_driver(drv);
> +       int ret;
>
>         /* When driver_override is set, only bind to the matching driver */
> -       if (pdev->driver_override)
> -               return !strcmp(pdev->driver_override, drv->name);
> +       ret = device_match_driver_override(dev, drv);
> +       if (ret >= 0)
> +               return ret;
>
>         /* Attempt an OF style match first */
>         if (of_driver_match_device(dev, drv))
> diff --git a/drivers/bus/simple-pm-bus.c b/drivers/bus/simple-pm-bus.c
> index 3f00d953fb9a..c920bd6fbaaf 100644
> --- a/drivers/bus/simple-pm-bus.c
> +++ b/drivers/bus/simple-pm-bus.c
> @@ -36,7 +36,7 @@ static int simple_pm_bus_probe(struct platform_device *pdev)
>          * that's not listed in simple_pm_bus_of_match. We don't want to do any
>          * of the simple-pm-bus tasks for these devices, so return early.
>          */
> -       if (pdev->driver_override)
> +       if (device_has_driver_override(&pdev->dev))
>                 return 0;
>
>         match = of_match_device(dev->driver->of_match_table, dev);
> @@ -78,7 +78,7 @@ static void simple_pm_bus_remove(struct platform_device *pdev)
>  {
>         const void *data = of_device_get_match_data(&pdev->dev);
>
> -       if (pdev->driver_override || data)
> +       if (device_has_driver_override(&pdev->dev) || data)
>                 return;
>
>         dev_dbg(&pdev->dev, "%s\n", __func__);
> diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
> index a85ec48a798b..9b33df9967ec 100644
> --- a/drivers/clk/imx/clk-scu.c
> +++ b/drivers/clk/imx/clk-scu.c
> @@ -706,8 +706,7 @@ struct clk_hw *imx_clk_scu_alloc_dev(const char *name,
>         if (ret)
>                 goto put_device;
>
> -       ret = driver_set_override(&pdev->dev, &pdev->driver_override,
> -                                 "imx-scu-clk", strlen("imx-scu-clk"));
> +       ret = device_set_driver_override(&pdev->dev, "imx-scu-clk");
>         if (ret)
>                 goto put_device;
>
> diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
> index 9aa7218b4e8d..1ed6be6e85d2 100644
> --- a/drivers/slimbus/qcom-ngd-ctrl.c
> +++ b/drivers/slimbus/qcom-ngd-ctrl.c
> @@ -1535,10 +1535,8 @@ static int of_qcom_slim_ngd_register(struct device *parent,
>                 ngd->id = id;
>                 ngd->pdev->dev.parent = parent;
>
> -               ret = driver_set_override(&ngd->pdev->dev,
> -                                         &ngd->pdev->driver_override,
> -                                         QCOM_SLIM_NGD_DRV_NAME,
> -                                         strlen(QCOM_SLIM_NGD_DRV_NAME));
> +               ret = device_set_driver_override(&ngd->pdev->dev,
> +                                                QCOM_SLIM_NGD_DRV_NAME);
>                 if (ret) {
>                         platform_device_put(ngd->pdev);
>                         kfree(ngd);
> diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
> index 813da101b5bf..ed1d50d1c3c1 100644
> --- a/include/linux/platform_device.h
> +++ b/include/linux/platform_device.h
> @@ -31,11 +31,6 @@ struct platform_device {
>         struct resource *resource;
>
>         const struct platform_device_id *id_entry;
> -       /*
> -        * Driver name to force a match.  Do not set directly, because core
> -        * frees it.  Use driver_set_override() to set or clear it.
> -        */
> -       const char *driver_override;
>
>         /* MFD cell pointer */
>         struct mfd_cell *mfd_cell;
> diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
> index e9964f0e010a..140907a41a70 100644
> --- a/sound/soc/samsung/i2s.c
> +++ b/sound/soc/samsung/i2s.c
> @@ -1360,10 +1360,10 @@ static int i2s_create_secondary_device(struct samsung_i2s_priv *priv)
>         if (!pdev_sec)
>                 return -ENOMEM;
>
> -       pdev_sec->driver_override = kstrdup("samsung-i2s", GFP_KERNEL);
> -       if (!pdev_sec->driver_override) {
> +       ret = device_set_driver_override(&pdev_sec->dev, "samsung-i2s");
> +       if (ret) {
>                 platform_device_put(pdev_sec);
> -               return -ENOMEM;
> +               return ret;
>         }
>
>         ret = platform_device_add(pdev_sec);
> --
> 2.53.0
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/3] driver core: generalize driver_override infrastructure
  2026-03-02  0:25 [PATCH 0/3] driver core: generalize driver_override infrastructure Danilo Krummrich
                   ` (2 preceding siblings ...)
  2026-03-02  0:25 ` [PATCH 3/3] driver core: platform: use generic driver_override infrastructure Danilo Krummrich
@ 2026-03-02  9:41 ` Gui-Dong Han
  2026-03-02 10:12   ` Danilo Krummrich
  3 siblings, 1 reply; 21+ messages in thread
From: Gui-Dong Han @ 2026-03-02  9:41 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].
>
> This patch series includes the migration of the platform bus; patches for all
> other affected busses still need to be extracted as a follow-up of the WIP
> treewide patch in [3].
>
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=220789
> [2] https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=driver_override
>
> Danilo Krummrich (3):
>   driver core: generalize driver_override in struct device
>   hwmon: axi-fan: don't use driver_override as IRQ name
>   driver core: platform: use generic driver_override infrastructure

Hi Danilo,

It looks like some usages of platform_device->driver_override were
missed. I found them here:
- drivers/bus/simple-pm-bus.c
- drivers/clk/imx/clk-scu.c
- drivers/slimbus/qcom-ngd-ctrl.c
- sound/soc/samsung/i2s.c

The good news is these can be easily updated to use the new APIs. This
is required to avoid breaking the build, since the field is removed
from struct platform_device. The previous build likely passed because
these weren't enabled. I will use allyesconfig for testing going
forward.

I scanned for similar cases and most fit the new APIs perfectly. One
exception is drivers/xen/xen-pciback/pci_stub.c. It does
strcmp(dev->driver_override, PCISTUB_DRIVER_NAME) instead of using
drv->name. We might want to change device_match_driver_override() to
take a const char * instead to handle this.

Besides axi-fan, I didn't find any other drivers that need to read
driver_override. This is great, as it means we hopefully won't need to
expose a read API at all.

Thanks.

>
>  arch/sh/drivers/platform_early.c |  6 ++-
>  drivers/base/core.c              |  2 +
>  drivers/base/dd.c                | 60 +++++++++++++++++++++++
>  drivers/base/platform.c          | 35 ++------------
>  drivers/bus/simple-pm-bus.c      |  4 +-
>  drivers/clk/imx/clk-scu.c        |  3 +-
>  drivers/hwmon/axi-fan-control.c  |  2 +-
>  drivers/slimbus/qcom-ngd-ctrl.c  |  6 +--
>  include/linux/device.h           | 81 ++++++++++++++++++++++++++++++++
>  include/linux/platform_device.h  |  5 --
>  sound/soc/samsung/i2s.c          |  6 +--
>  11 files changed, 161 insertions(+), 49 deletions(-)
>
>
> base-commit: 78437ab3b769f80526416570f60173c89858dd84
> --
> 2.53.0
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/3] hwmon: axi-fan: don't use driver_override as IRQ name
  2026-03-02  0:51   ` Guenter Roeck
@ 2026-03-02 10:00     ` Danilo Krummrich
  2026-03-02 11:02     ` Nuno Sá
  1 sibling, 0 replies; 21+ messages in thread
From: Danilo Krummrich @ 2026-03-02 10:00 UTC (permalink / raw)
  To: Guenter Roeck
  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, Guenter Roeck

On Mon Mar 2, 2026 at 1:51 AM CET, Guenter Roeck wrote:
> On 3/1/26 16:25, Danilo Krummrich wrote:
>> Do not use driver_override as IRQ name, as it is not guaranteed to point
>> to a valid string; use dev_name() instead.
>> 
>> Fixes: 8412b410fa5e ("hwmon: Support ADI Fan Control IP")
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>> ---
>>   drivers/hwmon/axi-fan-control.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/hwmon/axi-fan-control.c b/drivers/hwmon/axi-fan-control.c
>> index b7bb325c3ad9..ec4bbb104449 100644
>> --- a/drivers/hwmon/axi-fan-control.c
>> +++ b/drivers/hwmon/axi-fan-control.c
>> @@ -507,7 +507,7 @@ static int axi_fan_control_probe(struct platform_device *pdev)
>>   	ret = devm_request_threaded_irq(&pdev->dev, ctl->irq, NULL,
>>   					axi_fan_control_irq_handler,
>>   					IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
>> -					pdev->driver_override, ctl);
>> +					dev_name(&pdev->dev), ctl);
>
> The devm_request_threaded_irq() API documentation says:
>
> @devname:    An ascii name for the claiming device, dev_name(dev) if NULL
>
> So NULL should be sufficient.

I usually prefer to be explicit, but I can change it to NULL.

Thanks,
Danilo

^ 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  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 0/3] driver core: generalize driver_override infrastructure
  2026-03-02  9:41 ` [PATCH 0/3] driver core: generalize " Gui-Dong Han
@ 2026-03-02 10:12   ` Danilo Krummrich
  2026-03-02 10:59     ` Gui-Dong Han
  0 siblings, 1 reply; 21+ messages in thread
From: Danilo Krummrich @ 2026-03-02 10:12 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 10:41 AM CET, Gui-Dong Han wrote:
>> Danilo Krummrich (3):
>>   driver core: generalize driver_override in struct device
>>   hwmon: axi-fan: don't use driver_override as IRQ name
>>   driver core: platform: use generic driver_override infrastructure
>
> Hi Danilo,
>
> It looks like some usages of platform_device->driver_override were
> missed. I found them here:
> - drivers/bus/simple-pm-bus.c
> - drivers/clk/imx/clk-scu.c
> - drivers/slimbus/qcom-ngd-ctrl.c
> - sound/soc/samsung/i2s.c

They should all be covered by patch 3, no?

> The good news is these can be easily updated to use the new APIs. This
> is required to avoid breaking the build, since the field is removed
> from struct platform_device. The previous build likely passed because
> these weren't enabled. I will use allyesconfig for testing going
> forward.
>
> I scanned for similar cases and most fit the new APIs perfectly. One
> exception is drivers/xen/xen-pciback/pci_stub.c. It does
> strcmp(dev->driver_override, PCISTUB_DRIVER_NAME) instead of using
> drv->name. We might want to change device_match_driver_override() to
> take a const char * instead to handle this.

xen_pcibk_pci_driver should use the exact same define, so we can just convert
this to:

diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c
index e4b27aecbf0591..301207b4a30dac 100644
--- a/drivers/xen/xen-pciback/pci_stub.c
+++ b/drivers/xen/xen-pciback/pci_stub.c
@@ -609,9 +609,9 @@ 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)) ||
-	    match) {
+	if (device_match_driver_override(&dev->dev,
+					 &xen_pcibk_pci_driver.driver) > 0 ||
+					 match) {

 		if (dev->hdr_type != PCI_HEADER_TYPE_NORMAL
 		    && dev->hdr_type != PCI_HEADER_TYPE_BRIDGE) {

I.e. no separate API needed.

> Besides axi-fan, I didn't find any other drivers that need to read
> driver_override. This is great, as it means we hopefully won't need to
> expose a read API at all.

Great, thanks for checking.

^ 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  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: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 0/3] driver core: generalize driver_override infrastructure
  2026-03-02 10:12   ` Danilo Krummrich
@ 2026-03-02 10:59     ` Gui-Dong Han
  0 siblings, 0 replies; 21+ messages in thread
From: Gui-Dong Han @ 2026-03-02 10:59 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:12 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Mon Mar 2, 2026 at 10:41 AM CET, Gui-Dong Han wrote:
> >> Danilo Krummrich (3):
> >>   driver core: generalize driver_override in struct device
> >>   hwmon: axi-fan: don't use driver_override as IRQ name
> >>   driver core: platform: use generic driver_override infrastructure
> >
> > Hi Danilo,
> >
> > It looks like some usages of platform_device->driver_override were
> > missed. I found them here:
> > - drivers/bus/simple-pm-bus.c
> > - drivers/clk/imx/clk-scu.c
> > - drivers/slimbus/qcom-ngd-ctrl.c
> > - sound/soc/samsung/i2s.c
>
> They should all be covered by patch 3, no?

My apologies, I got a bit confused here.

>
> > The good news is these can be easily updated to use the new APIs. This
> > is required to avoid breaking the build, since the field is removed
> > from struct platform_device. The previous build likely passed because
> > these weren't enabled. I will use allyesconfig for testing going
> > forward.
> >
> > I scanned for similar cases and most fit the new APIs perfectly. One
> > exception is drivers/xen/xen-pciback/pci_stub.c. It does
> > strcmp(dev->driver_override, PCISTUB_DRIVER_NAME) instead of using
> > drv->name. We might want to change device_match_driver_override() to
> > take a const char * instead to handle this.
>
> xen_pcibk_pci_driver should use the exact same define, so we can just convert
> this to:
>
> diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c
> index e4b27aecbf0591..301207b4a30dac 100644
> --- a/drivers/xen/xen-pciback/pci_stub.c
> +++ b/drivers/xen/xen-pciback/pci_stub.c
> @@ -609,9 +609,9 @@ 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)) ||
> -           match) {
> +       if (device_match_driver_override(&dev->dev,
> +                                        &xen_pcibk_pci_driver.driver) > 0 ||
> +                                        match) {
>
>                 if (dev->hdr_type != PCI_HEADER_TYPE_NORMAL
>                     && dev->hdr_type != PCI_HEADER_TYPE_BRIDGE) {
>
> I.e. no separate API needed.

Thanks for the explanation, you are absolutely right.

>
> > Besides axi-fan, I didn't find any other drivers that need to read
> > driver_override. This is great, as it means we hopefully won't need to
> > expose a read API at all.
>
> Great, thanks for checking.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/3] hwmon: axi-fan: don't use driver_override as IRQ name
  2026-03-02  0:51   ` Guenter Roeck
  2026-03-02 10:00     ` Danilo Krummrich
@ 2026-03-02 11:02     ` Nuno Sá
  1 sibling, 0 replies; 21+ messages in thread
From: Nuno Sá @ 2026-03-02 11:02 UTC (permalink / raw)
  To: Guenter Roeck, 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

On Sun, 2026-03-01 at 16:51 -0800, Guenter Roeck wrote:
> On 3/1/26 16:25, Danilo Krummrich wrote:
> > Do not use driver_override as IRQ name, as it is not guaranteed to point
> > to a valid string; use dev_name() instead.
> > 
> > Fixes: 8412b410fa5e ("hwmon: Support ADI Fan Control IP")
> > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> > ---
> >   drivers/hwmon/axi-fan-control.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/hwmon/axi-fan-control.c b/drivers/hwmon/axi-fan-control.c
> > index b7bb325c3ad9..ec4bbb104449 100644
> > --- a/drivers/hwmon/axi-fan-control.c
> > +++ b/drivers/hwmon/axi-fan-control.c
> > @@ -507,7 +507,7 @@ static int axi_fan_control_probe(struct platform_device *pdev)
> >   	ret = devm_request_threaded_irq(&pdev->dev, ctl->irq, NULL,
> >   					axi_fan_control_irq_handler,
> >   					IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
> > -					pdev->driver_override, ctl);
> > +					dev_name(&pdev->dev), ctl);
> 
> The devm_request_threaded_irq() API documentation says:
> 
> @devname:    An ascii name for the claiming device, dev_name(dev) if NULL
> 
> So NULL should be sufficient.
> 
> Nuno, was there a special reason to use driver_override ?
> 
> 

Not really. That driver was one of my first patches so most likely it was just some
copy pasting. NULL should be fine.

- Nuno Sá

^ 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 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 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

end of thread, other threads:[~2026-03-02 16:29 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-02  0:25 [PATCH 0/3] driver core: generalize driver_override infrastructure Danilo Krummrich
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 11:04       ` Gui-Dong Han
2026-03-02 10:00   ` Geert Uytterhoeven
2026-03-02 10:26     ` Danilo Krummrich
2026-03-02 10:38       ` Geert Uytterhoeven
2026-03-02 11:03         ` Danilo Krummrich
2026-03-02 10:23   ` Armin Wolf
2026-03-02 16:28     ` Danilo Krummrich
2026-03-02  0:25 ` [PATCH 2/3] hwmon: axi-fan: don't use driver_override as IRQ name Danilo Krummrich
2026-03-02  0:51   ` Guenter Roeck
2026-03-02 10:00     ` Danilo Krummrich
2026-03-02 11:02     ` Nuno Sá
2026-03-02  0:25 ` [PATCH 3/3] driver core: platform: use generic driver_override infrastructure Danilo Krummrich
2026-03-02  8:55   ` Gui-Dong Han
2026-03-02  9:41 ` [PATCH 0/3] driver core: generalize " Gui-Dong Han
2026-03-02 10:12   ` Danilo Krummrich
2026-03-02 10:59     ` Gui-Dong Han

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox