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

Michael Kelley
2026-03-25T17:28:53Z

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
>