* [PATCH] hv/vmbus: copy dynamic IDs before dropping the lock
@ 2026-10-19 8:55 Jiale Yao
2026-10-19 13:35 ` Greg Kroah-Hartman
0 siblings, 1 reply; 2+ messages in thread
From: Jiale Yao @ 2026-10-19 8:55 UTC (permalink / raw)
To: Haiyang Zhang, Wei Liu, Dexuan Cui, Greg Kroah-Hartman,
Stephen Hemminger, K. Y. Srinivasan, linux-hyperv, linux-kernel
Cc: Jiale Yao
remove_id_store() can free a matching dynamic ID while VMBus is
preparing to invoke a driver probe. Copy the ID while holding the
dynamic ID lock so probe never receives a pointer into a freed list
entry.
Fixes: fc76936d3ea5 ("vmbus: add support for dynamic device id's")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
drivers/hv/vmbus_drv.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 4692687cdbf3..96ad0dfaa50b 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -661,21 +661,22 @@ hv_vmbus_dev_match(const struct hv_vmbus_device_id *id, const guid_t *guid)
}
static const struct hv_vmbus_device_id *
-hv_vmbus_dynid_match(struct hv_driver *drv, const guid_t *guid)
+hv_vmbus_dynid_match(struct hv_driver *drv, const guid_t *guid,
+ struct hv_vmbus_device_id *id_copy)
{
- const struct hv_vmbus_device_id *id = NULL;
struct vmbus_dynid *dynid;
spin_lock(&drv->dynids.lock);
list_for_each_entry(dynid, &drv->dynids.list, node) {
if (guid_equal(&dynid->id.guid, guid)) {
- id = &dynid->id;
- break;
+ *id_copy = dynid->id;
+ spin_unlock(&drv->dynids.lock);
+ return id_copy;
}
}
spin_unlock(&drv->dynids.lock);
- return id;
+ return NULL;
}
static const struct hv_vmbus_device_id vmbus_device_null;
@@ -685,7 +686,8 @@ static const struct hv_vmbus_device_id vmbus_device_null;
* If there is no match, return NULL.
*/
static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver *drv,
- struct hv_device *dev)
+ struct hv_device *dev,
+ struct hv_vmbus_device_id *id_copy)
{
const guid_t *guid = &dev->dev_type;
const struct hv_vmbus_device_id *id;
@@ -697,7 +699,7 @@ static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver *
return NULL;
/* Look at the dynamic ids first, before the static ones */
- id = hv_vmbus_dynid_match((struct hv_driver *)drv, guid);
+ id = hv_vmbus_dynid_match((struct hv_driver *)drv, guid, id_copy);
if (!id)
id = hv_vmbus_dev_match(drv->id_table, guid);
@@ -773,6 +775,7 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,
size_t count)
{
struct hv_driver *drv = drv_to_hv_drv(driver);
+ struct hv_vmbus_device_id id_copy;
guid_t guid;
ssize_t retval;
@@ -780,7 +783,7 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,
if (retval)
return retval;
- if (hv_vmbus_dynid_match(drv, &guid))
+ if (hv_vmbus_dynid_match(drv, &guid, &id_copy))
return -EEXIST;
retval = vmbus_add_dynid(drv, &guid);
@@ -840,12 +843,13 @@ static int vmbus_match(struct device *device, const struct device_driver *driver
{
const struct hv_driver *drv = drv_to_hv_drv(driver);
struct hv_device *hv_dev = device_to_hv_device(device);
+ struct hv_vmbus_device_id id_copy;
/* The hv_sock driver handles all hv_sock offers. */
if (is_hvsock_channel(hv_dev->channel))
return drv->hvsock;
- if (hv_vmbus_get_id(drv, hv_dev))
+ if (hv_vmbus_get_id(drv, hv_dev, &id_copy))
return 1;
return 0;
@@ -860,9 +864,10 @@ static int vmbus_probe(struct device *child_device)
struct hv_driver *drv =
drv_to_hv_drv(child_device->driver);
struct hv_device *dev = device_to_hv_device(child_device);
+ struct hv_vmbus_device_id dev_id_copy;
const struct hv_vmbus_device_id *dev_id;
- dev_id = hv_vmbus_get_id(drv, dev);
+ dev_id = hv_vmbus_get_id(drv, dev, &dev_id_copy);
if (drv->probe) {
ret = drv->probe(dev, dev_id);
if (ret != 0)
--
2.34.1
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [PATCH] hv/vmbus: copy dynamic IDs before dropping the lock
2026-10-19 8:55 [PATCH] hv/vmbus: copy dynamic IDs before dropping the lock Jiale Yao
@ 2026-10-19 13:35 ` Greg Kroah-Hartman
0 siblings, 0 replies; 2+ messages in thread
From: Greg Kroah-Hartman @ 2026-10-19 13:35 UTC (permalink / raw)
To: Jiale Yao
Cc: Haiyang Zhang, Wei Liu, Dexuan Cui, Stephen Hemminger,
K. Y. Srinivasan, linux-hyperv, linux-kernel
On Mon, Oct 19, 2026 at 04:55:44PM +0800, Jiale Yao wrote:
> remove_id_store() can free a matching dynamic ID while VMBus is
> preparing to invoke a driver probe.
And how can that be triggered?
> Copy the ID while holding the dynamic ID lock so probe never receives
> a pointer into a freed list entry.
Why is this driver allowed to handle dynamic ids at all? Is it really
required? That will taint the kernel so no real workflow requires it,
right?
> Fixes: fc76936d3ea5 ("vmbus: add support for dynamic device id's")
No cc: stable?
> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> ---
> drivers/hv/vmbus_drv.c | 25 +++++++++++++++----------
> 1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 4692687cdbf3..96ad0dfaa50b 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -661,21 +661,22 @@ hv_vmbus_dev_match(const struct hv_vmbus_device_id *id, const guid_t *guid)
> }
>
> static const struct hv_vmbus_device_id *
> -hv_vmbus_dynid_match(struct hv_driver *drv, const guid_t *guid)
> +hv_vmbus_dynid_match(struct hv_driver *drv, const guid_t *guid,
> + struct hv_vmbus_device_id *id_copy)
> {
> - const struct hv_vmbus_device_id *id = NULL;
> struct vmbus_dynid *dynid;
>
> spin_lock(&drv->dynids.lock);
> list_for_each_entry(dynid, &drv->dynids.list, node) {
> if (guid_equal(&dynid->id.guid, guid)) {
> - id = &dynid->id;
> - break;
> + *id_copy = dynid->id;
Shouldn't this be a memcpy() as that's what you are really doing?
> + spin_unlock(&drv->dynids.lock);
guard() usage instead?
> + return id_copy;
> }
> }
> spin_unlock(&drv->dynids.lock);
>
> - return id;
> + return NULL;
> }
>
> static const struct hv_vmbus_device_id vmbus_device_null;
> @@ -685,7 +686,8 @@ static const struct hv_vmbus_device_id vmbus_device_null;
> * If there is no match, return NULL.
> */
> static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver *drv,
> - struct hv_device *dev)
> + struct hv_device *dev,
> + struct hv_vmbus_device_id *id_copy)
> {
> const guid_t *guid = &dev->dev_type;
> const struct hv_vmbus_device_id *id;
> @@ -697,7 +699,7 @@ static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver *
> return NULL;
>
> /* Look at the dynamic ids first, before the static ones */
> - id = hv_vmbus_dynid_match((struct hv_driver *)drv, guid);
> + id = hv_vmbus_dynid_match((struct hv_driver *)drv, guid, id_copy);
> if (!id)
> id = hv_vmbus_dev_match(drv->id_table, guid);
>
> @@ -773,6 +775,7 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,
> size_t count)
> {
> struct hv_driver *drv = drv_to_hv_drv(driver);
> + struct hv_vmbus_device_id id_copy;
On the stack?
Ok, I guess this would work, but it feels clunky, why doesn't all other
busses have to do this too?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-19 13:35 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-19 8:55 [PATCH] hv/vmbus: copy dynamic IDs before dropping the lock Jiale Yao
2026-10-19 13:35 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®