The EHCI controller in QEMU seems to cause a UAF (Use-After-Free) issue when
handling packets from abnormal USB devices. Specifically, when the USB device's
firmware behaves abnormally and causes some initialization requests to block
and time out, passing that USB device through to QEMU's EHCI controller appears
to make the do-while loop in ehci_advance_state repeatedly submit multiple
requests to handle the same packet (this do-while loop is quite complex;
I confirmed the issue by adding logs in usb_host_cancel_packet).

When the virtual machine restarts, QEMU receives the IADD instruction issued
by the VM's kernel and will clean up in-flight packets in ehci_advance_async_state:
ehci_advance_async_state
└── ehci_queues_rip_unseen
    └── ehci_free_queue
        └── ehci_cancel_queue
            └── ehci_free_packet
                └── usb_cancel_packet
                    └── usb_device_cancel_packet
                        └── usb_host_cancel_packet

The cleanup actions in usb_host_cancel_packet mainly include:
1. Finding the first request in the device corresponding to the packet
2. Setting r->p of that request to NULL
3. Calling libusb_cancel_transfer for asynchronous cleanup

In the callback function usb_host_req_complete_ctrl registered to r->xfer, r->p
is checked, and if r->p is NULL, the corresponding handling is skipped. However,
since usb_host_cancel_packet only cleans up the first request handling the
corresponding packet, when there are multiple requests handling the same packet,
after the packet is cleared, other requests triggering the callback will cause
a UAF problem and trigger various QEMU cores.

We've verified that canceling all related requests when canceling a packet can
avoid this issue, but we haven't figured out why QEMU submits multiple requests
to handle the same packet. Do you have any suggestions? Thank you.

Reported by: yefenzheng1@h-partners.com

Signed-off-by: zoudongjie <zoudongjie@huawei.com>
---
 hw/usb/host-libusb.c | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index b74670ae25..b5aab12aee 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -406,18 +406,6 @@ static void usb_host_req_free(USBHostRequest *r)
     g_free(r);
 }
 
-static USBHostRequest *usb_host_req_find(USBHostDevice *s, USBPacket *p)
-{
-    USBHostRequest *r;
-
-    QTAILQ_FOREACH(r, &s->requests, next) {
-        if (r->p == p) {
-            return r;
-        }
-    }
-    return NULL;
-}
-
 static void LIBUSB_CALL usb_host_req_complete_ctrl(struct libusb_transfer *xfer)
 {
     USBHostRequest *r = xfer->user_data;
@@ -1276,7 +1264,7 @@ static void usb_host_unrealize(USBDevice *udev)
 static void usb_host_cancel_packet(USBDevice *udev, USBPacket *p)
 {
     USBHostDevice *s = USB_HOST_DEVICE(udev);
-    USBHostRequest *r;
+    USBHostRequest *r, *next_entry;
 
     if (p->combined) {
         usb_combined_packet_cancel(udev, p);
@@ -1285,10 +1273,17 @@ static void usb_host_cancel_packet(USBDevice *udev, USBPacket *p)
 
     trace_usb_host_req_canceled(s->bus_num, s->addr, p);
 
-    r = usb_host_req_find(s, p);
-    if (r && r->p) {
-        r->p = NULL; /* mark as dead */
-        libusb_cancel_transfer(r->xfer);
+    QTAILQ_FOREACH_SAFE(r, &s->requests, next, next_entry) {
+        if (r->p == p) {
+            if (unlikely(r && r->fake_in_flight)) {
+                usb_host_req_free(r);
+                continue;
+            }
+            if (r && r->p) {
+                r->p = NULL; /* mark as dead */
+                libusb_cancel_transfer(r->xfer);
+            }
+        }
     }
 }
 
-- 
2.33.0

On Tue, 4 Nov 2025 at 09:42, zoudongjie <zoudongjie@huawei.com> wrote:
>
> The EHCI controller in QEMU seems to cause a UAF (Use-After-Free) issue when
> handling packets from abnormal USB devices. Specifically, when the USB device's
> firmware behaves abnormally and causes some initialization requests to block
> and time out, passing that USB device through to QEMU's EHCI controller appears
> to make the do-while loop in ehci_advance_state repeatedly submit multiple
> requests to handle the same packet (this do-while loop is quite complex;
> I confirmed the issue by adding logs in usb_host_cancel_packet).

Hi; thanks for this patch. I'm not a USB expert so my comments
below are just some surface level questions.  Like you, I wonder
if the correct fix ought instead to be to not have multiple in-flight
requests for the same packet, but I don't know our USB subsystem
at all well enough to answer that.

> diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
> index b74670ae25..b5aab12aee 100644
> --- a/hw/usb/host-libusb.c
> +++ b/hw/usb/host-libusb.c
> @@ -406,18 +406,6 @@ static void usb_host_req_free(USBHostRequest *r)
>      g_free(r);
>  }
>
> -static USBHostRequest *usb_host_req_find(USBHostDevice *s, USBPacket *p)
> -{
> -    USBHostRequest *r;
> -
> -    QTAILQ_FOREACH(r, &s->requests, next) {
> -        if (r->p == p) {
> -            return r;
> -        }
> -    }
> -    return NULL;
> -}
> -
>  static void LIBUSB_CALL usb_host_req_complete_ctrl(struct libusb_transfer *xfer)
>  {
>      USBHostRequest *r = xfer->user_data;
> @@ -1276,7 +1264,7 @@ static void usb_host_unrealize(USBDevice *udev)
>  static void usb_host_cancel_packet(USBDevice *udev, USBPacket *p)
>  {
>      USBHostDevice *s = USB_HOST_DEVICE(udev);
> -    USBHostRequest *r;
> +    USBHostRequest *r, *next_entry;
>
>      if (p->combined) {
>          usb_combined_packet_cancel(udev, p);
> @@ -1285,10 +1273,17 @@ static void usb_host_cancel_packet(USBDevice *udev, USBPacket *p)
>
>      trace_usb_host_req_canceled(s->bus_num, s->addr, p);
>
> -    r = usb_host_req_find(s, p);
> -    if (r && r->p) {
> -        r->p = NULL; /* mark as dead */
> -        libusb_cancel_transfer(r->xfer);
> +    QTAILQ_FOREACH_SAFE(r, &s->requests, next, next_entry) {
> +        if (r->p == p) {
> +            if (unlikely(r && r->fake_in_flight)) {

What is this fake_in_flight field ? r is a USBHostRequest
but the definition of this struct doesn't seem to have that
field in it.

> +                usb_host_req_free(r);
> +                continue;
> +            }
> +            if (r && r->p) {

We're inside the "if (r->p == p)" check here, so we
know that r and r->p are the same. And the
QTAILQ_FOREACH_SAFE() macro that iterates 'r' through
the request list will only give us non-NULL pointers;
it terminates when it hits the NULL. So I don't think
we need to test this, it's always true.

> +                r->p = NULL; /* mark as dead */
> +                libusb_cancel_transfer(r->xfer);
> +            }
> +        }
>      }
>  }

thanks
-- PMM