RE: [PATCH 04/12] hv: vmbus: use generic driver_override infrastructure

[Michael Kelley <mhklinux@outlook.com>]

From: Danilo Krummrich <dakr@kernel.org> Sent: Monday, March 23, 2026 5:59 PM
> 

In the patch "Subject" line, the prefix for changes for vmbus_drv.c has
historically been "Drivers: hv: vmbus:".  It's a mouthful, but has been kept
fairly consistent over time.

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

I've tested this patch in a Hyper-V VM with VMBus devices. Did a simple
VMBus driver override, listed the overrides, and then removed the override.
All the right things happened with driver binding, unbind, etc.

Tested-by: Michael Kelley <mhklinux@outlook.com>

Modulo updates to the comments that I've noted below (and the patch
Subject line mentioned above):

Reviewed-by: Michael Kelley <mhklinux@outlook.com>

> 
> 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: d765edbb301c ("vmbus: add driver_override support")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  drivers/hv/vmbus_drv.c | 36 +++++-------------------------------
>  include/linux/hyperv.h |  5 -----
>  2 files changed, 5 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index bc4fc1951ae1..bc8dfd136f3c 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c

[snip]

> 
> @@ -711,9 +682,11 @@ static const struct hv_vmbus_device_id
> *hv_vmbus_get_id(const struct hv_driver *
>  {
>  	const guid_t *guid = &dev->dev_type;
>  	const struct hv_vmbus_device_id *id;
> +	int ret;
> 
>  	/* When driver_override is set, only bind to the matching driver */

This reference to "driver_override" in the comment was originally to the
"driver_override" field in struct hv_device, which has now gone away. Better
wording would be "If a driver override is set, only bind ...."

> -	if (dev->driver_override && strcmp(dev->driver_override, drv->name))
> +	ret = device_match_driver_override(&dev->device, &drv->driver);
> +	if (ret == 0)
>  		return NULL;
> 
>  	/* Look at the dynamic ids first, before the static ones */
> @@ -722,7 +695,7 @@ static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver *
>  		id = hv_vmbus_dev_match(drv->id_table, guid);
> 
>  	/* driver_override will always match, send a dummy id */

Again, the reference to "driver_override" no longer makes sense. The
original comment is a bit opaque in its own way. Let me suggest this new
wording:

If there's a matching driver override, this function should succeed. So
return a dummy device ID if no matching ID is found.

> -	if (!id && dev->driver_override)
> +	if (!id && ret > 0)
>  		id = &vmbus_device_null;
> 
>  	return id;
> @@ -1024,6 +997,7 @@ static const struct dev_pm_ops vmbus_pm = {
>  /* The one and only one */
>  static const struct bus_type  hv_bus = {
>  	.name =		"vmbus",
> +	.driver_override =	true,
>  	.match =		vmbus_match,
>  	.shutdown =		vmbus_shutdown,
>  	.remove =		vmbus_remove,
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index dfc516c1c719..bf689d07d750 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1272,11 +1272,6 @@ struct hv_device {
>  	u16 device_id;
> 
>  	struct device device;
> -	/*
> -	 * Driver name to force a match.  Do not set directly, because core
> -	 * frees it.  Use driver_set_override() to set or clear it.
> -	 */
> -	const char *driver_override;
> 
>  	struct vmbus_channel *channel;
>  	struct kset	     *channels_kset;
> --
> 2.53.0
>