From: Danilo Krummrich <[email protected]> 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 <[email protected]> Modulo updates to the comments that I've noted below (and the patch Subject line mentioned above): Reviewed-by: Michael Kelley <[email protected]> > > Link: > https://lore.kernel.org/driver-core/[email protected]/ [1] > Reported-by: Gui-Dong Han <[email protected]> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220789 > Fixes: d765edbb301c ("vmbus: add driver_override support") > Signed-off-by: Danilo Krummrich <[email protected]> > --- > 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 >
