[PATCH v2 0/2] Drivers: hv: vmbus: fix dynamic ID races

[PATCH v2 0/2] Drivers: hv: vmbus: fix dynamic ID races

[Jiale Yao]

VMBus dynamic IDs have two races with their sysfs add and remove paths.

First, new_id_store() checks for an existing ID separately from inserting
the new entry, allowing concurrent writers to add duplicate GUIDs. Move the
check and insertion into one critical section.

Second, matching returns a pointer into the dynamic ID list after dropping
the lock. A concurrent remove_id write can free that entry before probe uses
it. Copy the ID while holding the lock and reject probe when a dynamic ID is
removed between matching and probing.

The series was found by static analysis and checked with checkpatch.pl. Each
patch was also checked individually and in series for application to the
base commit below.

Jiale Yao (2):
  Drivers: hv: vmbus: prevent duplicate dynamic IDs
  Drivers: hv: vmbus: copy dynamic IDs before dropping the lock

---
Changes in v2:
- Split the dynamic ID insertion TOCTOU into a preparatory patch.
- Reject probe when the matching ID disappears before the second lookup.
- Preserve the ID-less hvsock probe path.
- Use scoped locking and memcpy() for the dynamic ID copy.
- Document that the device ID is valid only during probe.
- Cite the corresponding PCI and USB dynamic ID lifetime fixes.
- Add Cc: stable@vger.kernel.org and use the VMBus subject prefix.

Link: https://lore.kernel.org/r/20261019085544.1834700-1-yaojiale02@163.com

 drivers/hv/vmbus_drv.c | 56 +++++++++++++++++++++++++-----------------
 include/linux/hyperv.h |  1 +
 2 files changed, 34 insertions(+), 23 deletions(-)


base-commit: a804c5742bff224d4cb4d25dd383554d598be32b
-- 
2.34.1

[PATCH v2 1/2] Drivers: hv: vmbus: prevent duplicate dynamic IDs

[Jiale Yao]

new_id_store() checks for an existing dynamic ID before calling
vmbus_add_dynid(), but the check and insertion use separate critical
sections. Two concurrent writers can therefore both observe that a GUID is
absent and add duplicate entries. Removing the GUID once then leaves the
duplicate active.

Move the duplicate check into vmbus_add_dynid() so it is performed under
the same lock as the list insertion.

Fixes: fc76936d3ea5 ("vmbus: add support for dynamic device id's")
Cc: stable@vger.kernel.org
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/hv/vmbus_drv.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 4692687cdbf3..1037117e2d81 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -737,7 +737,7 @@ static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver *
  */
 static int vmbus_add_dynid(struct hv_driver *drv, guid_t *guid)
 {
-	struct vmbus_dynid *dynid;
+	struct vmbus_dynid *dynid, *existing;
 
 	dynid = kzalloc_obj(*dynid);
 	if (!dynid)
@@ -745,9 +745,16 @@ static int vmbus_add_dynid(struct hv_driver *drv, guid_t *guid)
 
 	dynid->id.guid = *guid;
 
-	spin_lock(&drv->dynids.lock);
-	list_add_tail(&dynid->node, &drv->dynids.list);
-	spin_unlock(&drv->dynids.lock);
+	scoped_guard(spinlock, &drv->dynids.lock) {
+		list_for_each_entry(existing, &drv->dynids.list, node) {
+			if (guid_equal(&existing->id.guid, guid)) {
+				kfree(dynid);
+				return -EEXIST;
+			}
+		}
+
+		list_add_tail(&dynid->node, &drv->dynids.list);
+	}
 
 	return driver_attach(&drv->driver);
 }
@@ -780,9 +787,6 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,
 	if (retval)
 		return retval;
 
-	if (hv_vmbus_dynid_match(drv, &guid))
-		return -EEXIST;
-
 	retval = vmbus_add_dynid(drv, &guid);
 	if (retval)
 		return retval;
-- 
2.34.1

[PATCH v2 2/2] Drivers: hv: vmbus: copy dynamic IDs before dropping the lock

[Jiale Yao]

Dynamic IDs are freed by remove_id_store() while holding dynids.lock, so a
pointer into a dynamic ID entry is valid only while that lock is held.
hv_vmbus_dynid_match() currently returns such a pointer after unlocking. A
concurrent remove_id write can therefore free the entry before a driver
probe dereferences it.

Copy a matching dynamic ID under the lock and pass the caller-owned copy to
the probe callback. This follows the lifetime handling added for PCI by
commit 3ffc4c9690c3 ("PCI: Fix UAF when probe runs concurrent to dyn ID
removal") and for USB by commit ef8154d8b52d ("usb: fix UAF when probe runs
concurrent to dyn ID removal"). Document that the ID is valid only for the
duration of probe.

Because matching and probing perform separate lookups, removal can also
happen between them. Reject the probe if the second lookup no longer finds
an ID instead of passing NULL to drivers which dereference it. Keep the
hvsock exception because it does not use an ID table or inspect the ID.

This issue was found by static analysis of the dynamic ID lifetime.

Fixes: fc76936d3ea5 ("vmbus: add support for dynamic device id's")
Cc: stable@vger.kernel.org
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/hv/vmbus_drv.c | 38 ++++++++++++++++++++++----------------
 include/linux/hyperv.h |  1 +
 2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 1037117e2d81..cc439d9295fa 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -661,21 +661,21 @@ 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;
+	scoped_guard(spinlock, &drv->dynids.lock) {
+		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;
+		list_for_each_entry(dynid, &drv->dynids.list, node) {
+			if (guid_equal(&dynid->id.guid, guid)) {
+				memcpy(id_copy, &dynid->id, sizeof(*id_copy));
+				return id_copy;
+			}
 		}
 	}
-	spin_unlock(&drv->dynids.lock);
 
-	return id;
+	return NULL;
 }
 
 static const struct hv_vmbus_device_id vmbus_device_null;
@@ -685,7 +685,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 +698,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);
 
@@ -843,13 +844,14 @@ ATTRIBUTE_GROUPS(vmbus_drv);
 static int vmbus_match(struct device *device, const struct device_driver *driver)
 {
 	const struct hv_driver *drv = drv_to_hv_drv(driver);
+	struct hv_vmbus_device_id id_copy;
 	struct hv_device *hv_dev = device_to_hv_device(device);
 
 	/* 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,13 +862,17 @@ static int vmbus_match(struct device *device, const struct device_driver *driver
  */
 static int vmbus_probe(struct device *child_device)
 {
-	int ret = 0;
+	const struct hv_vmbus_device_id *dev_id;
+	struct hv_vmbus_device_id dev_id_copy;
 	struct hv_driver *drv =
 			drv_to_hv_drv(child_device->driver);
 	struct hv_device *dev = device_to_hv_device(child_device);
-	const struct hv_vmbus_device_id *dev_id;
+	int ret = 0;
+
+	dev_id = hv_vmbus_get_id(drv, dev, &dev_id_copy);
+	if (!dev_id && !is_hvsock_channel(dev->channel))
+		return -ENODEV;
 
-	dev_id = hv_vmbus_get_id(drv, dev);
 	if (drv->probe) {
 		ret = drv->probe(dev, dev_id);
 		if (ret != 0)
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 9e109d91aa14..0f0423e8a184 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1264,6 +1264,7 @@ struct hv_driver {
 		struct list_head list;
 	} dynids;
 
+	/* The device ID pointer is only valid during the probe callback. */
 	int (*probe)(struct hv_device *, const struct hv_vmbus_device_id *);
 	void (*remove)(struct hv_device *dev);
 	void (*shutdown)(struct hv_device *);
-- 
2.34.1

Re: [PATCH v2 2/2] Drivers: hv: vmbus: copy dynamic IDs before dropping the lock

[Kameron Carr]

On 10/21/2026 7:33 AM, Jiale Yao wrote:
[...]
> @@ -860,13 +862,17 @@ static int vmbus_match(struct device *device, const struct device_driver *driver
>   */
>  static int vmbus_probe(struct device *child_device)
>  {
> -	int ret = 0;
> +	const struct hv_vmbus_device_id *dev_id;
> +	struct hv_vmbus_device_id dev_id_copy;
>  	struct hv_driver *drv =
>  			drv_to_hv_drv(child_device->driver);
>  	struct hv_device *dev = device_to_hv_device(child_device);
> -	const struct hv_vmbus_device_id *dev_id;
> +	int ret = 0;

Nit: the reordering of these declarations is unnecessary.
In a stable patch, we shouldn't include any purely style changes.

-- 
Regards,
Kameron

> +
> +	dev_id = hv_vmbus_get_id(drv, dev, &dev_id_copy);
> +	if (!dev_id && !is_hvsock_channel(dev->channel))
> +		return -ENODEV;
>  
> -	dev_id = hv_vmbus_get_id(drv, dev);
>  	if (drv->probe) {
>  		ret = drv->probe(dev, dev_id);
>  		if (ret != 0)
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 9e109d91aa14..0f0423e8a184 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1264,6 +1264,7 @@ struct hv_driver {
>  		struct list_head list;
>  	} dynids;
>  
> +	/* The device ID pointer is only valid during the probe callback. */
>  	int (*probe)(struct hv_device *, const struct hv_vmbus_device_id *);
>  	void (*remove)(struct hv_device *dev);
>  	void (*shutdown)(struct hv_device *);

Re: [PATCH v2 0/2] Drivers: hv: vmbus: fix dynamic ID races

[Kameron Carr]

On 10/21/2026 7:33 AM, Jiale Yao wrote:
> VMBus dynamic IDs have two races with their sysfs add and remove paths.
> 
> First, new_id_store() checks for an existing ID separately from inserting
> the new entry, allowing concurrent writers to add duplicate GUIDs. Move the
> check and insertion into one critical section.
> 
> Second, matching returns a pointer into the dynamic ID list after dropping
> the lock. A concurrent remove_id write can free that entry before probe uses
> it. Copy the ID while holding the lock and reject probe when a dynamic ID is
> removed between matching and probing.
> 
> The series was found by static analysis and checked with checkpatch.pl. Each
> patch was also checked individually and in series for application to the
> base commit below.
> 
> Jiale Yao (2):
>   Drivers: hv: vmbus: prevent duplicate dynamic IDs
>   Drivers: hv: vmbus: copy dynamic IDs before dropping the lock
> 
> ---
> Changes in v2:
> - Split the dynamic ID insertion TOCTOU into a preparatory patch.
> - Reject probe when the matching ID disappears before the second lookup.
> - Preserve the ID-less hvsock probe path.
> - Use scoped locking and memcpy() for the dynamic ID copy.
> - Document that the device ID is valid only during probe.
> - Cite the corresponding PCI and USB dynamic ID lifetime fixes.
> - Add Cc: stable@vger.kernel.org and use the VMBus subject prefix.
> 
> Link: https://lore.kernel.org/r/20261019085544.1834700-1-yaojiale02@163.com
> 
>  drivers/hv/vmbus_drv.c | 56 +++++++++++++++++++++++++-----------------
>  include/linux/hyperv.h |  1 +
>  2 files changed, 34 insertions(+), 23 deletions(-)
> 
> 
> base-commit: a804c5742bff224d4cb4d25dd383554d598be32b

Performed regression testing in Azure. No issues detected.

Tested-by: Kameron Carr <kameroncarr@linux.microsoft.com>


Besides the one NIT in PATCH 2/2, for the full series,

Reviewed-by: Kameron Carr <kameroncarr@linux.microsoft.com>

-- 
Regards,
Kameron