hw/usb/hcd-uhci: don't assert for SETUP to non-0 endpoint

[PATCH] hw/usb/hcd-uhci: don't assert for SETUP to non-0 endpoint

Posted by Peter Maydell 1 week, 6 days ago

If the guest feeds invalid data to the UHCI controller, we
can assert:
qemu-system-x86_64: ../../hw/usb/core.c:744: usb_ep_get: Assertion `pid == USB_TOKEN_IN || pid == USB_TOKEN_OUT' failed.

(see issue 2548 for the repro case).  This happens because the guest
attempts USB_TOKEN_SETUP to an endpoint other than 0, which is not
valid.  The controller code doesn't catch this guest error, so
instead we hit the assertion in the USB core code.

Catch the case of SETUP to non-zero endpoint, and treat it as a fatal
error in the TD, in the same way we do for an invalid PID value in
the TD.

This is the UHCI equivalent of the same bug in OHCI that we fixed in
commit 3c3c233677 ("hw/usb/hcd-ohci: Fix #1510, #303: pid not IN or
OUT").

This bug has been tracked as CVE-2024-8354.

Cc: qemu-stable@nongnu.org
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2548
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/usb/hcd-uhci.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 4822c704f69..e207d0587a1 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -735,6 +735,7 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
     bool spd;
     bool queuing = (q != NULL);
     uint8_t pid = td->token & 0xff;
+    uint8_t ep_id = (td->token >> 15) & 0xf;
     UHCIAsync *async;
 
     async = uhci_async_find_td(s, td_addr);
@@ -778,9 +779,14 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
 
     switch (pid) {
     case USB_TOKEN_OUT:
-    case USB_TOKEN_SETUP:
     case USB_TOKEN_IN:
         break;
+    case USB_TOKEN_SETUP:
+        /* SETUP is only valid to endpoint 0 */
+        if (ep_id == 0) {
+            break;
+        }
+        /* fallthrough */
     default:
         /* invalid pid : frame interrupted */
         s->status |= UHCI_STS_HCPERR;
@@ -829,7 +835,7 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
             return uhci_handle_td_error(s, td, td_addr, USB_RET_NODEV,
                                         int_mask);
         }
-        ep = usb_ep_get(dev, pid, (td->token >> 15) & 0xf);
+        ep = usb_ep_get(dev, pid, ep_id);
         q = uhci_queue_new(s, qh_addr, td, ep);
     }
     async = uhci_async_alloc(q, td_addr);
-- 
2.43.0

Re: [PATCH] hw/usb/hcd-uhci: don't assert for SETUP to non-0 endpoint

Posted by Michael Tokarev 1 week, 4 days ago

On 15.09.2025 16:29, Peter Maydell wrote:
> If the guest feeds invalid data to the UHCI controller, we
> can assert:
> qemu-system-x86_64: ../../hw/usb/core.c:744: usb_ep_get: Assertion `pid == USB_TOKEN_IN || pid == USB_TOKEN_OUT' failed.
> 
> (see issue 2548 for the repro case).  This happens because the guest
> attempts USB_TOKEN_SETUP to an endpoint other than 0, which is not
> valid.  The controller code doesn't catch this guest error, so
> instead we hit the assertion in the USB core code.
> 
> Catch the case of SETUP to non-zero endpoint, and treat it as a fatal
> error in the TD, in the same way we do for an invalid PID value in
> the TD.
> 
> This is the UHCI equivalent of the same bug in OHCI that we fixed in
> commit 3c3c233677 ("hw/usb/hcd-ohci: Fix #1510, #303: pid not IN or
> OUT").
> 
> This bug has been tracked as CVE-2024-8354.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2548
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>

Thank you for this Peter!

/mjt

Re: [PATCH] hw/usb/hcd-uhci: don't assert for SETUP to non-0 endpoint

Posted by Peter Maydell 3 days, 5 hours ago

On Wed, 17 Sept 2025 at 15:03, Michael Tokarev <mjt@tls.msk.ru> wrote:
>
> On 15.09.2025 16:29, Peter Maydell wrote:
> > If the guest feeds invalid data to the UHCI controller, we
> > can assert:
> > qemu-system-x86_64: ../../hw/usb/core.c:744: usb_ep_get: Assertion `pid == USB_TOKEN_IN || pid == USB_TOKEN_OUT' failed.
> >
> > (see issue 2548 for the repro case).  This happens because the guest
> > attempts USB_TOKEN_SETUP to an endpoint other than 0, which is not
> > valid.  The controller code doesn't catch this guest error, so
> > instead we hit the assertion in the USB core code.
> >
> > Catch the case of SETUP to non-zero endpoint, and treat it as a fatal
> > error in the TD, in the same way we do for an invalid PID value in
> > the TD.
> >
> > This is the UHCI equivalent of the same bug in OHCI that we fixed in
> > commit 3c3c233677 ("hw/usb/hcd-ohci: Fix #1510, #303: pid not IN or
> > OUT").
> >
> > This bug has been tracked as CVE-2024-8354.
> >
> > Cc: qemu-stable@nongnu.org
> > Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2548
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>

Thanks; I'll queue this via target-arm.next unless anybody
objects to that.

-- PMM