Linux kernel -stable discussions
 help / color / mirror / Atom feed
* [PATCH] driver core: enforce device_lock for driver_match_device()
@ 2026-06-02 16:08 Runyu Xiao
  2026-06-02 16:39 ` Greg KH
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Runyu Xiao @ 2026-06-02 16:08 UTC (permalink / raw)
  To: gregkh
  Cc: rafael, dakr, cornelia.huck, tom.leiming, linux-kernel, stable,
	jianhao.xu, runyu.xiao

Currently driver_match_device() is called from three sites. The
__device_attach_driver() path already runs under device_lock(dev), but
bind_store() and __driver_attach() can still enter bus match()
callbacks without that lock held.

That inconsistency leaves bus-private driver_override readers exposed.
Several buses still read private driver_override strings from their
match callbacks while the write side relies on driver_set_override()
under device_lock(dev). If bind_store() or __driver_attach() reaches
such a match callback without that lock, it can race with
driver_override replacement and old-string free.

This issue was first flagged by our static analysis tool while auditing
driver_override match paths, then manually confirmed on Linux v6.18.21.
We reproduced the race with no-device KCSAN/MSV harnesses across AMBA,
WMI, RPMSG, VMBUS, VDPA, CDX, CSS, FSL-MC, and PCI. Those reports all
reduce to the same core-side gap in driver_match_device().

Fix this by introducing driver_match_device_locked(), which guarantees
holding device_lock(dev) with a scoped guard before entering the bus
match callback. Convert the two unlocked call sites to this helper, and
add a device_lock_assert() to driver_match_device() so the contract is
explicit.

This matches the locking requirement already assumed by
driver_set_override()-based driver_override implementations and avoids
trying to paper over the problem in each individual bus match()
callback, where taking device_lock(dev) locally would conflict with
already-locked callers.

Build-tested with:
  make olddefconfig
  make -j"$(nproc)" drivers/base/bus.o drivers/base/dd.o

Fixes: 49b420a13ff9 ("driver core: check bus->match without holding device lock")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 drivers/base/base.h | 10 ++++++++++
 drivers/base/bus.c  |  2 +-
 drivers/base/dd.c   |  2 +-
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/base/base.h b/drivers/base/base.h
index 86fa7fbb3548..4ec487e34d1a 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -166,9 +166,19 @@ void device_set_deferred_probe_reason(const struct device *dev, struct va_format
 static inline int driver_match_device(const struct device_driver *drv,
 				      struct device *dev)
 {
+	device_lock_assert(dev);
+
 	return drv->bus->match ? drv->bus->match(dev, drv) : 1;
 }
 
+static inline int driver_match_device_locked(const struct device_driver *drv,
+					     struct device *dev)
+{
+	guard(device)(dev);
+
+	return driver_match_device(drv, dev);
+}
+
 static inline void dev_sync_state(struct device *dev)
 {
 	if (dev->bus->sync_state)
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 2653670f962f..2b039aa2da74 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -263,7 +263,7 @@ static ssize_t bind_store(struct device_driver *drv, const char *buf,
 	int err = -ENODEV;
 
 	dev = bus_find_device_by_name(bus, NULL, buf);
-	if (dev && driver_match_device(drv, dev)) {
+	if (dev && driver_match_device_locked(drv, dev)) {
 		err = device_driver_attach(drv, dev);
 		if (!err) {
 			/* success */
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 2996f4c667c4..30c3e55ff5ff 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -1230,7 +1230,7 @@ static int __driver_attach(struct device *dev, void *data)
 	 * is an error.
 	 */
 
-	ret = driver_match_device(drv, dev);
+	ret = driver_match_device_locked(drv, dev);
 	if (ret == 0) {
 		/* no match */
 		return 0;
-- 
2.34.1

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

* Re: [PATCH] driver core: enforce device_lock for driver_match_device()
  2026-06-02 16:08 [PATCH] driver core: enforce device_lock for driver_match_device() Runyu Xiao
@ 2026-06-02 16:39 ` Greg KH
  2026-06-02 16:44 ` Danilo Krummrich
       [not found] ` <20260604035239.1711889-1-runyu.xiao@seu.edu.cn>
  2 siblings, 0 replies; 9+ messages in thread
From: Greg KH @ 2026-06-02 16:39 UTC (permalink / raw)
  To: Runyu Xiao
  Cc: rafael, dakr, cornelia.huck, tom.leiming, linux-kernel, stable,
	jianhao.xu

On Wed, Jun 03, 2026 at 12:08:29AM +0800, Runyu Xiao wrote:
> Currently driver_match_device() is called from three sites. The
> __device_attach_driver() path already runs under device_lock(dev), but
> bind_store() and __driver_attach() can still enter bus match()
> callbacks without that lock held.
> 
> That inconsistency leaves bus-private driver_override readers exposed.
> Several buses still read private driver_override strings from their
> match callbacks while the write side relies on driver_set_override()
> under device_lock(dev). If bind_store() or __driver_attach() reaches
> such a match callback without that lock, it can race with
> driver_override replacement and old-string free.
> 
> This issue was first flagged by our static analysis tool while auditing
> driver_override match paths, then manually confirmed on Linux v6.18.21.

That is very old, please test on the latest 7.1-rc release as things
have changed in this area recently.

thanks,

greg k-h

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

* Re: [PATCH] driver core: enforce device_lock for driver_match_device()
  2026-06-02 16:08 [PATCH] driver core: enforce device_lock for driver_match_device() Runyu Xiao
  2026-06-02 16:39 ` Greg KH
@ 2026-06-02 16:44 ` Danilo Krummrich
  2026-06-03  3:36   ` Runyu Xiao
       [not found] ` <20260604035239.1711889-1-runyu.xiao@seu.edu.cn>
  2 siblings, 1 reply; 9+ messages in thread
From: Danilo Krummrich @ 2026-06-02 16:44 UTC (permalink / raw)
  To: Runyu Xiao
  Cc: gregkh, rafael, cornelia.huck, tom.leiming, linux-kernel, stable,
	jianhao.xu

On Tue Jun 2, 2026 at 6:08 PM CEST, Runyu Xiao wrote:
> This issue was first flagged by our static analysis tool while auditing
> driver_override match paths, then manually confirmed on Linux v6.18.21.
> We reproduced the race with no-device KCSAN/MSV harnesses across AMBA,
> WMI, RPMSG, VMBUS, VDPA, CDX, CSS, FSL-MC, and PCI. Those reports all
> reduce to the same core-side gap in driver_match_device().

This is all fixed in driver-core-next already, please also see [1] and [2].

> Fix this by introducing driver_match_device_locked(), which guarantees
> holding device_lock(dev) with a scoped guard before entering the bus
> match callback. Convert the two unlocked call sites to this helper, and
> add a device_lock_assert() to driver_match_device() so the contract is
> explicit.

This approach was reverted [3] for the reasons documented in the linked patch.

Thanks,
Danilo

[1] https://lore.kernel.org/driver-core/20260303115720.48783-1-dakr@kernel.org/
[2] https://lore.kernel.org/driver-core/20260324005919.2408620-1-dakr@kernel.org/
[3] https://lore.kernel.org/driver-core/20260302002545.19389-1-dakr@kernel.org/

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

* Re: [PATCH] driver core: enforce device_lock for driver_match_device()
  2026-06-02 16:44 ` Danilo Krummrich
@ 2026-06-03  3:36   ` Runyu Xiao
  0 siblings, 0 replies; 9+ messages in thread
From: Runyu Xiao @ 2026-06-03  3:36 UTC (permalink / raw)
  To: dakr
  Cc: Runyu Xiao, gregkh, rafael, cornelia.huck, tom.leiming,
	linux-kernel, stable, jianhao.xu

Hi Danilo, Greg,

Thanks for the pointers.

Understood. Since this is already being addressed in driver-core-next
through the generic driver_override infrastructure, and the
device_lock(driver_match_device()) approach was reverted for the reasons
you pointed out, I will not resend this patch.

I will retest the original issue on the latest tree and only follow up
again if there is still a remaining problem with the current
driver_override infrastructure.

Thanks,
Runyu

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

* [PATCH v2 1/4] amba: use generic driver_override infrastructure
       [not found] ` <20260604035239.1711889-1-runyu.xiao@seu.edu.cn>
@ 2026-06-04  3:52   ` Runyu Xiao
  2026-06-04  3:52   ` [PATCH v2 2/4] rpmsg: core: " Runyu Xiao
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Runyu Xiao @ 2026-06-04  3:52 UTC (permalink / raw)
  To: gregkh, rafael
  Cc: dakr, driver-core, linux, andersson, mathieu.poirier, kys,
	haiyangz, wei.liu, decui, longli, nipun.gupta, nikhil.agarwal,
	linux-remoteproc, linux-arm-msm, linux-hyperv, linux-kernel,
	jianhao.xu, runyu.xiao, stable

AMBA devices still keep driver_override in bus-private storage.

The sysfs write side updates that string through driver_set_override(),
which replaces the pointer and frees the old value. However,
driver_match_device() can call amba_match() from __driver_attach()
without holding the device lock, and amba_match() still dereferences
that private pointer directly.

That means a bind/unbind or reprobe path can race with a concurrent
driver_override update and make amba_match() compare against freed
memory.

Fix this by switching AMBA to the driver-core driver_override
infrastructure. This lets the core own the sysfs attribute and storage,
and uses device_match_driver_override() for the locked read in the match
path.

Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/
Fixes: 3cf385713460 ("ARM: 8256/1: driver coamba: add device binding path 'driver_override'")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
drivers/amba/bus.c       | 35 +++++------------------------------
include/linux/amba/bus.h |  5 -----
 2 files changed, 5 insertions(+), 35 deletions(-)

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 6d479caf89cb..df8333f90906 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -82,33 +82,6 @@ static void amba_put_disable_pclk(struct amba_device *pcdev)
 }
 
 
-static ssize_t driver_override_show(struct device *_dev,
-				    struct device_attribute *attr, char *buf)
-{
-	struct amba_device *dev = to_amba_device(_dev);
-	ssize_t len;
-
-	device_lock(_dev);
-	len = sprintf(buf, "%s\n", dev->driver_override);
-	device_unlock(_dev);
-	return len;
-}
-
-static ssize_t driver_override_store(struct device *_dev,
-				     struct device_attribute *attr,
-				     const char *buf, size_t count)
-{
-	struct amba_device *dev = to_amba_device(_dev);
-	int ret;
-
-	ret = driver_set_override(_dev, &dev->driver_override, buf, count);
-	if (ret)
-		return ret;
-
-	return count;
-}
-static DEVICE_ATTR_RW(driver_override);
-
 #define amba_attr_func(name,fmt,arg...)					\
 static ssize_t name##_show(struct device *_dev,				\
 			   struct device_attribute *attr, char *buf)	\
@@ -126,7 +99,6 @@ amba_attr_func(resource, "\t%016llx\t%016llx\t%016lx\n",
 static struct attribute *amba_dev_attrs[] = {
 	&dev_attr_id.attr,
 	&dev_attr_resource.attr,
-	&dev_attr_driver_override.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(amba_dev);
@@ -209,6 +181,7 @@ static int amba_match(struct device *dev, const struct device_driver *drv)
 {
 	struct amba_device *pcdev = to_amba_device(dev);
 	const struct amba_driver *pcdrv = to_amba_driver(drv);
+	int ret;
 
 	mutex_lock(&pcdev->periphid_lock);
 	if (!pcdev->periphid) {
@@ -230,8 +203,9 @@ static int amba_match(struct device *dev, const struct device_driver *drv)
 	mutex_unlock(&pcdev->periphid_lock);
 
 	/* When driver_override is set, only bind to the matching driver */
-	if (pcdev->driver_override)
-		return !strcmp(pcdev->driver_override, drv->name);
+	ret = device_match_driver_override(dev, drv);
+	if (ret >= 0)
+		return ret;
 
 	return amba_lookup(pcdrv->id_table, pcdev) != NULL;
 }
@@ -435,6 +409,7 @@ static const struct dev_pm_ops amba_pm = {
  */
 const struct bus_type amba_bustype = {
 	.name		= "amba",
+	.driver_override = true,
 	.dev_groups	= amba_dev_groups,
 	.match		= amba_match,
 	.uevent		= amba_uevent,
diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
index 9946276aff73..6c54d5c0d21f 100644
--- a/include/linux/amba/bus.h
+++ b/include/linux/amba/bus.h
@@ -71,11 +71,6 @@ struct amba_device {
 	unsigned int		cid;
 	struct amba_cs_uci_id	uci;
 	unsigned int		irq[AMBA_NR_IRQS];
-	/*
-	 * Driver name to force a match.  Do not set directly, because core
-	 * frees it.  Use driver_set_override() to set or clear it.
-	 */
-	const char		*driver_override;
 };
 
 struct amba_driver {
-- 
2.34.1

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

* [PATCH v2 2/4] rpmsg: core: use generic driver_override infrastructure
       [not found] ` <20260604035239.1711889-1-runyu.xiao@seu.edu.cn>
  2026-06-04  3:52   ` [PATCH v2 1/4] amba: use generic driver_override infrastructure Runyu Xiao
@ 2026-06-04  3:52   ` Runyu Xiao
  2026-06-08 17:47     ` Mathieu Poirier
  2026-06-04  3:52   ` [PATCH v2 3/4] vmbus: " Runyu Xiao
  2026-06-04  3:52   ` [PATCH v2 4/4] cdx: " Runyu Xiao
  3 siblings, 1 reply; 9+ messages in thread
From: Runyu Xiao @ 2026-06-04  3:52 UTC (permalink / raw)
  To: gregkh, rafael
  Cc: dakr, driver-core, linux, andersson, mathieu.poirier, kys,
	haiyangz, wei.liu, decui, longli, nipun.gupta, nikhil.agarwal,
	linux-remoteproc, linux-arm-msm, linux-hyperv, linux-kernel,
	jianhao.xu, runyu.xiao, stable

RPMSG still keeps driver_override in bus-private storage.

That private pointer can be updated from the sysfs driver_override
attribute, and also from rpmsg_register_device_override(). Both paths
replace the pointer and can free the old value.

However, driver_match_device() can call rpmsg_dev_match() from
__driver_attach() without holding the device lock, and rpmsg_dev_match()
still dereferences that private pointer directly.

This leaves the match path racing with concurrent driver_override
updates, with the usual risk of comparing against freed memory.

Switch rpmsg to the driver-core driver_override infrastructure. This
removes the private storage, uses device_match_driver_override() for the
locked read in rpmsg_dev_match(), and converts
rpmsg_register_device_override() to device_set_driver_override() so the
in-kernel override path uses the same core-managed storage. With that
storage now owned by struct device, drop the remaining rpmsg transport
release-path frees of rpdev->driver_override as well.

Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/
Fixes: 39e47767ec9b ("rpmsg: Add driver_override device attribute for rpmsg_device")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
drivers/rpmsg/qcom_glink_native.c |  2 --
drivers/rpmsg/rpmsg_core.c        | 41 ++++++--------------------------------
drivers/rpmsg/virtio_rpmsg_bus.c  |  1 -
include/linux/rpmsg.h             |  4 ----
 4 files changed, 6 insertions(+), 42 deletions(-)

diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index e7f7831d37f8..11d3007db5cd 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -358,33 +358,6 @@ rpmsg_show_attr(src, src, "0x%x\n");
 rpmsg_show_attr(dst, dst, "0x%x\n");
 rpmsg_show_attr(announce, announce ? "true" : "false", "%s\n");
 
-static ssize_t driver_override_store(struct device *dev,
-				     struct device_attribute *attr,
-				     const char *buf, size_t count)
-{
-	struct rpmsg_device *rpdev = to_rpmsg_device(dev);
-	int ret;
-
-	ret = driver_set_override(dev, &rpdev->driver_override, buf, count);
-	if (ret)
-		return ret;
-
-	return count;
-}
-
-static ssize_t driver_override_show(struct device *dev,
-				    struct device_attribute *attr, char *buf)
-{
-	struct rpmsg_device *rpdev = to_rpmsg_device(dev);
-	ssize_t len;
-
-	device_lock(dev);
-	len = sysfs_emit(buf, "%s\n", rpdev->driver_override);
-	device_unlock(dev);
-	return len;
-}
-static DEVICE_ATTR_RW(driver_override);
-
 static ssize_t modalias_show(struct device *dev,
 			     struct device_attribute *attr, char *buf)
 {
@@ -405,7 +378,6 @@ static struct attribute *rpmsg_dev_attrs[] = {
 	&dev_attr_dst.attr,
 	&dev_attr_src.attr,
 	&dev_attr_announce.attr,
-	&dev_attr_driver_override.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(rpmsg_dev);
@@ -424,9 +396,11 @@ static int rpmsg_dev_match(struct device *dev, const struct device_driver *drv)
 	const struct rpmsg_driver *rpdrv = to_rpmsg_driver(drv);
 	const struct rpmsg_device_id *ids = rpdrv->id_table;
 	unsigned int i;
+	int ret;
 
-	if (rpdev->driver_override)
-		return !strcmp(rpdev->driver_override, drv->name);
+	ret = device_match_driver_override(dev, drv);
+	if (ret >= 0)
+		return ret;
 
 	if (ids)
 		for (i = 0; ids[i].name[0]; i++)
@@ -533,6 +507,7 @@ static void rpmsg_dev_remove(struct device *dev)
 
 static const struct bus_type rpmsg_bus = {
 	.name		= "rpmsg",
+	.driver_override = true,
 	.match		= rpmsg_dev_match,
 	.dev_groups	= rpmsg_dev_groups,
 	.uevent		= rpmsg_uevent,
@@ -560,9 +535,7 @@ int rpmsg_register_device_override(struct rpmsg_device *rpdev,
 
 	device_initialize(dev);
 	if (driver_override) {
-		ret = driver_set_override(dev, &rpdev->driver_override,
-					  driver_override,
-					  strlen(driver_override));
+		ret = device_set_driver_override(dev, driver_override);
 		if (ret) {
 			dev_err(dev, "device_set_override failed: %d\n", ret);
 			put_device(dev);
@@ -573,8 +546,6 @@ int rpmsg_register_device_override(struct rpmsg_device *rpdev,
 	ret = device_add(dev);
 	if (ret) {
 		dev_err(dev, "device_add failed: %d\n", ret);
-		kfree(rpdev->driver_override);
-		rpdev->driver_override = NULL;
 		put_device(dev);
 	}
 
diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index 401a4ece0c97..d9d4468e4cbd 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -1626,7 +1626,6 @@ static void qcom_glink_rpdev_release(struct device *dev)
 {
 	struct rpmsg_device *rpdev = to_rpmsg_device(dev);
 
-	kfree(rpdev->driver_override);
 	kfree(rpdev);
 }
 
@@ -1862,7 +1861,6 @@ static void qcom_glink_device_release(struct device *dev)
 
 	/* Release qcom_glink_alloc_channel() reference */
 	kref_put(&channel->refcount, qcom_glink_channel_release);
-	kfree(rpdev->driver_override);
 	kfree(rpdev);
 }
 
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 5ae15111fb4f..1b8bb05924af 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -374,7 +374,6 @@ static void virtio_rpmsg_release_device(struct device *dev)
 	struct rpmsg_device *rpdev = to_rpmsg_device(dev);
 	struct virtio_rpmsg_channel *vch = to_virtio_rpmsg_channel(rpdev);
 
-	kfree(rpdev->driver_override);
 	kfree(vch);
 }
 
diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
index 83266ce14642..2e40eb54155e 100644
--- a/include/linux/rpmsg.h
+++ b/include/linux/rpmsg.h
@@ -41,9 +41,6 @@ struct rpmsg_channel_info {
  * rpmsg_device - device that belong to the rpmsg bus
  * @dev: the device struct
  * @id: device id (used to match between rpmsg drivers and devices)
- * @driver_override: driver name to force a match; do not set directly,
- *                   because core frees it; use driver_set_override() to
- *                   set or clear it.
  * @src: local address
  * @dst: destination address
  * @ept: the rpmsg endpoint of this channel
@@ -53,7 +50,6 @@ struct rpmsg_channel_info {
 struct rpmsg_device {
 	struct device dev;
 	struct rpmsg_device_id id;
-	const char *driver_override;
 	u32 src;
 	u32 dst;
 	struct rpmsg_endpoint *ept;
-- 
2.34.1

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

* [PATCH v2 3/4] vmbus: use generic driver_override infrastructure
       [not found] ` <20260604035239.1711889-1-runyu.xiao@seu.edu.cn>
  2026-06-04  3:52   ` [PATCH v2 1/4] amba: use generic driver_override infrastructure Runyu Xiao
  2026-06-04  3:52   ` [PATCH v2 2/4] rpmsg: core: " Runyu Xiao
@ 2026-06-04  3:52   ` Runyu Xiao
  2026-06-04  3:52   ` [PATCH v2 4/4] cdx: " Runyu Xiao
  3 siblings, 0 replies; 9+ messages in thread
From: Runyu Xiao @ 2026-06-04  3:52 UTC (permalink / raw)
  To: gregkh, rafael
  Cc: dakr, driver-core, linux, andersson, mathieu.poirier, kys,
	haiyangz, wei.liu, decui, longli, nipun.gupta, nikhil.agarwal,
	linux-remoteproc, linux-arm-msm, linux-hyperv, linux-kernel,
	jianhao.xu, runyu.xiao, stable

VMBUS devices still keep driver_override in bus-private storage.

The sysfs write side updates that string through driver_set_override(),
which replaces the pointer and frees the old value. However,
driver_match_device() can call into hv_vmbus_get_id() from
__driver_attach() without holding the device lock, and hv_vmbus_get_id()
still dereferences that private pointer directly.

That means a bind/reprobe path can race with a concurrent
driver_override update and make the match logic inspect freed memory.

Switch vmbus to the driver-core driver_override infrastructure. This
removes the private driver_override storage and uses
device_match_driver_override() for the locked read in the match path.

Keep the existing vmbus semantics intact: if driver_override matches but
no dynamic or static device ID matches, continue to return the dummy
vmbus_device_null ID so override-only binding still works as before.

Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/
Fixes: d765edbb301c ("vmbus: add driver_override support")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
drivers/hv/vmbus_drv.c | 36 +++++-------------------------------
include/linux/hyperv.h |  6 ------
 2 files changed, 5 insertions(+), 37 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index d28ff45d4cfd..a81e2b097636 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -538,34 +538,6 @@ static ssize_t device_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(device);
 
-static ssize_t driver_override_store(struct device *dev,
-				     struct device_attribute *attr,
-				     const char *buf, size_t count)
-{
-	struct hv_device *hv_dev = device_to_hv_device(dev);
-	int ret;
-
-	ret = driver_set_override(dev, &hv_dev->driver_override, buf, count);
-	if (ret)
-		return ret;
-
-	return count;
-}
-
-static ssize_t driver_override_show(struct device *dev,
-				    struct device_attribute *attr, char *buf)
-{
-	struct hv_device *hv_dev = device_to_hv_device(dev);
-	ssize_t len;
-
-	device_lock(dev);
-	len = sysfs_emit(buf, "%s\n", hv_dev->driver_override);
-	device_unlock(dev);
-
-	return len;
-}
-static DEVICE_ATTR_RW(driver_override);
-
 /* Set up per device attributes in /sys/bus/vmbus/devices/<bus device> */
 static struct attribute *vmbus_dev_attrs[] = {
 	&dev_attr_id.attr,
@@ -596,7 +568,6 @@ static struct attribute *vmbus_dev_attrs[] = {
 	&dev_attr_channel_vp_mapping.attr,
 	&dev_attr_vendor.attr,
 	&dev_attr_device.attr,
-	&dev_attr_driver_override.attr,
 	NULL,
 };
 
@@ -708,9 +679,11 @@ static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver *
 {
 	const guid_t *guid = &dev->dev_type;
 	const struct hv_vmbus_device_id *id;
+	int ret;
 
 	/* When driver_override is set, only bind to the matching driver */
-	if (dev->driver_override && strcmp(dev->driver_override, drv->name))
+	ret = device_match_driver_override(&dev->device, &drv->driver);
+	if (ret == 0)
 		return NULL;
 
 	/* Look at the dynamic ids first, before the static ones */
@@ -719,7 +692,7 @@ static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver *
 		id = hv_vmbus_dev_match(drv->id_table, guid);
 
 	/* driver_override will always match, send a dummy id */
-	if (!id && dev->driver_override)
+	if (!id && ret > 0)
 		id = &vmbus_device_null;
 
 	return id;
@@ -1021,6 +994,7 @@ static const struct dev_pm_ops vmbus_pm = {
 /* The one and only one */
 static const struct bus_type  hv_bus = {
 	.name =		"vmbus",
+	.driver_override = true,
 	.match =		vmbus_match,
 	.shutdown =		vmbus_shutdown,
 	.remove =		vmbus_remove,
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 964f1be8150c..f9ede569602d 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1272,12 +1272,6 @@ struct hv_device {
 	u16 device_id;
 
 	struct device device;
-	/*
-	 * Driver name to force a match.  Do not set directly, because core
-	 * frees it.  Use driver_set_override() to set or clear it.
-	 */
-	const char *driver_override;
-
 	struct vmbus_channel *channel;
 	struct kset	     *channels_kset;
 	struct device_dma_parameters dma_parms;
-- 
2.34.1

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

* [PATCH v2 4/4] cdx: use generic driver_override infrastructure
       [not found] ` <20260604035239.1711889-1-runyu.xiao@seu.edu.cn>
                     ` (2 preceding siblings ...)
  2026-06-04  3:52   ` [PATCH v2 3/4] vmbus: " Runyu Xiao
@ 2026-06-04  3:52   ` Runyu Xiao
  3 siblings, 0 replies; 9+ messages in thread
From: Runyu Xiao @ 2026-06-04  3:52 UTC (permalink / raw)
  To: gregkh, rafael
  Cc: dakr, driver-core, linux, andersson, mathieu.poirier, kys,
	haiyangz, wei.liu, decui, longli, nipun.gupta, nikhil.agarwal,
	linux-remoteproc, linux-arm-msm, linux-hyperv, linux-kernel,
	jianhao.xu, runyu.xiao, stable

CDX devices still keep driver_override in bus-private storage.

The sysfs write side updates that string through driver_set_override(),
which replaces the pointer and frees the old value. However,
driver_match_device() can call cdx_bus_match() from __driver_attach()
without holding the device lock, and cdx_bus_match() still dereferences
that private pointer directly.

That means the CDX match path can race with a concurrent
driver_override update and compare against freed memory.

Switch CDX to the driver-core driver_override infrastructure. This
removes the private driver_override storage, lets the core provide the
sysfs attribute, and uses device_match_driver_override() for the locked
read in cdx_bus_match().

Preserve the existing CDX override_only semantics: entries marked
override_only still require a matching driver_override, but ordinary ID
matches continue to work unchanged.

Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/
Fixes: 48a6c7bced2a ("cdx: add device attributes")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
drivers/cdx/cdx.c           | 40 +++++--------------------------------
include/linux/cdx/cdx_bus.h |  1 -
 2 files changed, 5 insertions(+), 36 deletions(-)

diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
index 9196dc50a48d..d3d230247262 100644
--- a/drivers/cdx/cdx.c
+++ b/drivers/cdx/cdx.c
@@ -156,8 +156,6 @@ static int cdx_unregister_device(struct device *dev,
 	} else {
 		cdx_destroy_res_attr(cdx_dev, MAX_CDX_DEV_RESOURCES);
 		debugfs_remove_recursive(cdx_dev->debugfs_dir);
-		kfree(cdx_dev->driver_override);
-		cdx_dev->driver_override = NULL;
 	}
 
 	/*
@@ -268,6 +266,7 @@ static int cdx_bus_match(struct device *dev, const struct device_driver *drv)
 	const struct cdx_driver *cdx_drv = to_cdx_driver(drv);
 	const struct cdx_device_id *found_id = NULL;
 	const struct cdx_device_id *ids;
+	int ret;
 
 	if (cdx_dev->is_bus)
 		return false;
@@ -275,7 +274,8 @@ static int cdx_bus_match(struct device *dev, const struct device_driver *drv)
 	ids = cdx_drv->match_id_table;
 
 	/* When driver_override is set, only bind to the matching driver */
-	if (cdx_dev->driver_override && strcmp(cdx_dev->driver_override, drv->name))
+	ret = device_match_driver_override(dev, drv);
+	if (ret == 0)
 		return false;
 
 	found_id = cdx_match_id(ids, cdx_dev);
@@ -289,7 +289,7 @@ static int cdx_bus_match(struct device *dev, const struct device_driver *drv)
 		 */
 		if (!found_id->override_only)
 			return true;
-		if (cdx_dev->driver_override)
+		if (ret > 0)
 			return true;
 
 		ids = found_id + 1;
@@ -453,36 +453,6 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(modalias);
 
-static ssize_t driver_override_store(struct device *dev,
-				     struct device_attribute *attr,
-				     const char *buf, size_t count)
-{
-	struct cdx_device *cdx_dev = to_cdx_device(dev);
-	int ret;
-
-	if (WARN_ON(dev->bus != &cdx_bus_type))
-		return -EINVAL;
-
-	ret = driver_set_override(dev, &cdx_dev->driver_override, buf, count);
-	if (ret)
-		return ret;
-
-	return count;
-}
-
-static ssize_t driver_override_show(struct device *dev,
-				    struct device_attribute *attr, char *buf)
-{
-	struct cdx_device *cdx_dev = to_cdx_device(dev);
-	ssize_t len;
-
-	device_lock(dev);
-	len = sysfs_emit(buf, "%s\n", cdx_dev->driver_override);
-	device_unlock(dev);
-	return len;
-}
-static DEVICE_ATTR_RW(driver_override);
-
 static ssize_t enable_store(struct device *dev, struct device_attribute *attr,
 			    const char *buf, size_t count)
 {
@@ -552,7 +522,6 @@ static struct attribute *cdx_dev_attrs[] = {
 	&dev_attr_class.attr,
 	&dev_attr_revision.attr,
 	&dev_attr_modalias.attr,
-	&dev_attr_driver_override.attr,
 	NULL,
 };
 
@@ -646,6 +615,7 @@ ATTRIBUTE_GROUPS(cdx_bus);
 
 const struct bus_type cdx_bus_type = {
 	.name		= "cdx",
+	.driver_override = true,
 	.match		= cdx_bus_match,
 	.probe		= cdx_probe,
 	.remove		= cdx_remove,
diff --git a/include/linux/cdx/cdx_bus.h b/include/linux/cdx/cdx_bus.h
index b1ba97f6c9ad..f1a107b232da 100644
--- a/include/linux/cdx/cdx_bus.h
+++ b/include/linux/cdx/cdx_bus.h
@@ -165,7 +165,6 @@ struct cdx_device {
 	bool enabled;
 	u32 msi_dev_id;
 	u32 num_msi;
-	const char *driver_override;
 	struct mutex irqchip_lock;
 	bool msi_write_pending;
 };
-- 
2.34.1

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

* Re: [PATCH v2 2/4] rpmsg: core: use generic driver_override infrastructure
  2026-06-04  3:52   ` [PATCH v2 2/4] rpmsg: core: " Runyu Xiao
@ 2026-06-08 17:47     ` Mathieu Poirier
  0 siblings, 0 replies; 9+ messages in thread
From: Mathieu Poirier @ 2026-06-08 17:47 UTC (permalink / raw)
  To: Runyu Xiao
  Cc: gregkh, rafael, dakr, driver-core, linux, andersson, kys,
	haiyangz, wei.liu, decui, longli, nipun.gupta, nikhil.agarwal,
	linux-remoteproc, linux-arm-msm, linux-hyperv, linux-kernel,
	jianhao.xu, stable

On Wed, 3 Jun 2026 at 21:52, Runyu Xiao <runyu.xiao@seu.edu.cn> wrote:
>
> RPMSG still keeps driver_override in bus-private storage.
>
> That private pointer can be updated from the sysfs driver_override
> attribute, and also from rpmsg_register_device_override(). Both paths
> replace the pointer and can free the old value.
>
> However, driver_match_device() can call rpmsg_dev_match() from
> __driver_attach() without holding the device lock, and rpmsg_dev_match()
> still dereferences that private pointer directly.
>
> This leaves the match path racing with concurrent driver_override
> updates, with the usual risk of comparing against freed memory.
>
> Switch rpmsg to the driver-core driver_override infrastructure. This
> removes the private storage, uses device_match_driver_override() for the
> locked read in rpmsg_dev_match(), and converts
> rpmsg_register_device_override() to device_set_driver_override() so the
> in-kernel override path uses the same core-managed storage. With that
> storage now owned by struct device, drop the remaining rpmsg transport
> release-path frees of rpdev->driver_override as well.
>
> Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/
> Fixes: 39e47767ec9b ("rpmsg: Add driver_override device attribute for rpmsg_device")
> Cc: stable@vger.kernel.org
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
> drivers/rpmsg/qcom_glink_native.c |  2 --
> drivers/rpmsg/rpmsg_core.c        | 41 ++++++--------------------------------
> drivers/rpmsg/virtio_rpmsg_bus.c  |  1 -
> include/linux/rpmsg.h             |  4 ----

For the bottom 3:

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

>  4 files changed, 6 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
> index e7f7831d37f8..11d3007db5cd 100644
> --- a/drivers/rpmsg/rpmsg_core.c
> +++ b/drivers/rpmsg/rpmsg_core.c
> @@ -358,33 +358,6 @@ rpmsg_show_attr(src, src, "0x%x\n");
>  rpmsg_show_attr(dst, dst, "0x%x\n");
>  rpmsg_show_attr(announce, announce ? "true" : "false", "%s\n");
>
> -static ssize_t driver_override_store(struct device *dev,
> -                                    struct device_attribute *attr,
> -                                    const char *buf, size_t count)
> -{
> -       struct rpmsg_device *rpdev = to_rpmsg_device(dev);
> -       int ret;
> -
> -       ret = driver_set_override(dev, &rpdev->driver_override, buf, count);
> -       if (ret)
> -               return ret;
> -
> -       return count;
> -}
> -
> -static ssize_t driver_override_show(struct device *dev,
> -                                   struct device_attribute *attr, char *buf)
> -{
> -       struct rpmsg_device *rpdev = to_rpmsg_device(dev);
> -       ssize_t len;
> -
> -       device_lock(dev);
> -       len = sysfs_emit(buf, "%s\n", rpdev->driver_override);
> -       device_unlock(dev);
> -       return len;
> -}
> -static DEVICE_ATTR_RW(driver_override);
> -
>  static ssize_t modalias_show(struct device *dev,
>                              struct device_attribute *attr, char *buf)
>  {
> @@ -405,7 +378,6 @@ static struct attribute *rpmsg_dev_attrs[] = {
>         &dev_attr_dst.attr,
>         &dev_attr_src.attr,
>         &dev_attr_announce.attr,
> -       &dev_attr_driver_override.attr,
>         NULL,
>  };
>  ATTRIBUTE_GROUPS(rpmsg_dev);
> @@ -424,9 +396,11 @@ static int rpmsg_dev_match(struct device *dev, const struct device_driver *drv)
>         const struct rpmsg_driver *rpdrv = to_rpmsg_driver(drv);
>         const struct rpmsg_device_id *ids = rpdrv->id_table;
>         unsigned int i;
> +       int ret;
>
> -       if (rpdev->driver_override)
> -               return !strcmp(rpdev->driver_override, drv->name);
> +       ret = device_match_driver_override(dev, drv);
> +       if (ret >= 0)
> +               return ret;
>
>         if (ids)
>                 for (i = 0; ids[i].name[0]; i++)
> @@ -533,6 +507,7 @@ static void rpmsg_dev_remove(struct device *dev)
>
>  static const struct bus_type rpmsg_bus = {
>         .name           = "rpmsg",
> +       .driver_override = true,
>         .match          = rpmsg_dev_match,
>         .dev_groups     = rpmsg_dev_groups,
>         .uevent         = rpmsg_uevent,
> @@ -560,9 +535,7 @@ int rpmsg_register_device_override(struct rpmsg_device *rpdev,
>
>         device_initialize(dev);
>         if (driver_override) {
> -               ret = driver_set_override(dev, &rpdev->driver_override,
> -                                         driver_override,
> -                                         strlen(driver_override));
> +               ret = device_set_driver_override(dev, driver_override);
>                 if (ret) {
>                         dev_err(dev, "device_set_override failed: %d\n", ret);
>                         put_device(dev);
> @@ -573,8 +546,6 @@ int rpmsg_register_device_override(struct rpmsg_device *rpdev,
>         ret = device_add(dev);
>         if (ret) {
>                 dev_err(dev, "device_add failed: %d\n", ret);
> -               kfree(rpdev->driver_override);
> -               rpdev->driver_override = NULL;
>                 put_device(dev);
>         }
>
> diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
> index 401a4ece0c97..d9d4468e4cbd 100644
> --- a/drivers/rpmsg/qcom_glink_native.c
> +++ b/drivers/rpmsg/qcom_glink_native.c
> @@ -1626,7 +1626,6 @@ static void qcom_glink_rpdev_release(struct device *dev)
>  {
>         struct rpmsg_device *rpdev = to_rpmsg_device(dev);
>
> -       kfree(rpdev->driver_override);
>         kfree(rpdev);
>  }
>
> @@ -1862,7 +1861,6 @@ static void qcom_glink_device_release(struct device *dev)
>
>         /* Release qcom_glink_alloc_channel() reference */
>         kref_put(&channel->refcount, qcom_glink_channel_release);
> -       kfree(rpdev->driver_override);
>         kfree(rpdev);
>  }
>
> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
> index 5ae15111fb4f..1b8bb05924af 100644
> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> @@ -374,7 +374,6 @@ static void virtio_rpmsg_release_device(struct device *dev)
>         struct rpmsg_device *rpdev = to_rpmsg_device(dev);
>         struct virtio_rpmsg_channel *vch = to_virtio_rpmsg_channel(rpdev);
>
> -       kfree(rpdev->driver_override);
>         kfree(vch);
>  }
>
> diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
> index 83266ce14642..2e40eb54155e 100644
> --- a/include/linux/rpmsg.h
> +++ b/include/linux/rpmsg.h
> @@ -41,9 +41,6 @@ struct rpmsg_channel_info {
>   * rpmsg_device - device that belong to the rpmsg bus
>   * @dev: the device struct
>   * @id: device id (used to match between rpmsg drivers and devices)
> - * @driver_override: driver name to force a match; do not set directly,
> - *                   because core frees it; use driver_set_override() to
> - *                   set or clear it.
>   * @src: local address
>   * @dst: destination address
>   * @ept: the rpmsg endpoint of this channel
> @@ -53,7 +50,6 @@ struct rpmsg_channel_info {
>  struct rpmsg_device {
>         struct device dev;
>         struct rpmsg_device_id id;
> -       const char *driver_override;
>         u32 src;
>         u32 dst;
>         struct rpmsg_endpoint *ept;
> --
> 2.34.1

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

end of thread, other threads:[~2026-06-08 17:47 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-02 16:08 [PATCH] driver core: enforce device_lock for driver_match_device() Runyu Xiao
2026-06-02 16:39 ` Greg KH
2026-06-02 16:44 ` Danilo Krummrich
2026-06-03  3:36   ` Runyu Xiao
     [not found] ` <20260604035239.1711889-1-runyu.xiao@seu.edu.cn>
2026-06-04  3:52   ` [PATCH v2 1/4] amba: use generic driver_override infrastructure Runyu Xiao
2026-06-04  3:52   ` [PATCH v2 2/4] rpmsg: core: " Runyu Xiao
2026-06-08 17:47     ` Mathieu Poirier
2026-06-04  3:52   ` [PATCH v2 3/4] vmbus: " Runyu Xiao
2026-06-04  3:52   ` [PATCH v2 4/4] cdx: " Runyu Xiao

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