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: [email protected]
Signed-off-by: zoudongjie <[email protected]>
---
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
