linux-kernel - Re:[PATCH] usb: xhci: Fix xhci_free_virt_devices_depth

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <446082a4.7dbe.199098cd654.Coremail.00107082@163.com>
Date: Tue, 2 Sep 2025 16:30:48 +0800 (CST)
From: "David Wang" <00107082@....com>
To: "Michal Pecio" <michal.pecio@...il.com>
Cc: "Mathias Nyman" <mathias.nyman@...ux.intel.com>,
	WeitaoWang-oc@...oxin.com, gregkh@...uxfoundation.org,
	linux-usb@...r.kernel.org, regressions@...ts.linux.dev,
	linux-kernel@...r.kernel.org, surenb@...gle.com,
	kent.overstreet@...ux.dev
Subject: Re:[PATCH] usb: xhci: Fix xhci_free_virt_devices_depth_first()

At 2025-09-02 15:30:17, "Michal Pecio" <michal.pecio@...il.com> wrote:
>Reusing 'vdev' for iteration caused a recent commit to malfunction
>uexpectedly, resulting in a reported memory leak and potential UAF
>if devices are freed in bad order. Using a second variable solves
>this problem, and maybe others later.
>
>HCS_MAX_SLOTS(xhci->hcs_params1) is the highest possible slot_id,
>so change the iteration range to include it. Currently this doesn't
>seem to cause problems because the only caller begins with freeing
>the topmost slot_id, but it breaks documented functionality.
>
>Reported-by: David Wang <00107082@....com>
>Closes: https://lore.kernel.org/linux-usb/20250829181354.4450-1-00107082@163.com/
>Fixes: 2eb03376151b ("usb: xhci: Fix slot_id resource race conflict")
>Fixes: ee8665e28e8d ("xhci: free xhci virtual devices with leaf nodes first")
>Cc: stable@...r.kernel.org
>Signed-off-by: Michal Pecio <michal.pecio@...il.com>
>---
> drivers/usb/host/xhci-mem.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
>index eed5926b200e..db7dc70c37e5 100644
>--- a/drivers/usb/host/xhci-mem.c
>+++ b/drivers/usb/host/xhci-mem.c
>@@ -932,7 +932,7 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, struct xhci_virt_device *dev,
>  */
> static void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int slot_id)
> {
>-	struct xhci_virt_device *vdev;
>+	struct xhci_virt_device *vdev, *vdev_i;
> 	struct list_head *tt_list_head;
> 	struct xhci_tt_bw_info *tt_info, *next;
> 	int i;
>@@ -951,9 +951,9 @@ static void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int slot_i
> 		/* is this a hub device that added a tt_info to the tts list */
> 		if (tt_info->slot_id == slot_id) {
> 			/* are any devices using this tt_info? */
>-			for (i = 1; i < HCS_MAX_SLOTS(xhci->hcs_params1); i++) {
>-				vdev = xhci->devs[i];
>-				if (vdev && (vdev->tt_info == tt_info))
>+			for (i = 1; i <= HCS_MAX_SLOTS(xhci->hcs_params1); i++) {
>+				vdev_i = xhci->devs[i];
>+				if (vdev_i && (vdev_i->tt_info == tt_info))
> 					xhci_free_virt_devices_depth_first(
> 						xhci, i);
> 			}
>-- 
>2.48.1

Tested-by: David Wang <00107082@....com>


About the change from "<" to "<=", I did not observe any difference on my system. Is it because my system does not use up all slots?

Thanks
David