[PATCH] hw/usb/hcd-xhci: Fix guest-triggerable assert() in xhci_find_stream()

Thomas Huth posted 1 patch 1 week, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260715203357.424556-1-thuth@redhat.com
hw/usb/hcd-xhci.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
[PATCH] hw/usb/hcd-xhci: Fix guest-triggerable assert() in xhci_find_stream()
Posted by Thomas Huth 1 week, 2 days ago
From: Thomas Huth <thuth@redhat.com>

The assert() statement in xhci_find_stream() can be triggered by
the guest (see bug tickets #273, #3895 and #3988 on gitlab.com).
Turn it into a qemu_log_mask() instead to fix this problem.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/273
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/usb/hcd-xhci.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index c7e050e38fd..ae8add4227e 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1009,7 +1009,12 @@ static XHCIStreamContext *xhci_find_stream(XHCIEPContext *epctx,
     dma_addr_t base;
     uint32_t ctx[2], sct;
 
-    assert(streamid != 0);
+    if (!streamid) {
+        qemu_log_mask(LOG_GUEST_ERROR, "xhci: stream ID is zero\n");
+        *cc_error = CC_INVALID_STREAM_ID_ERROR;
+        return NULL;
+    }
+
     if (epctx->lsa) {
         if (streamid >= epctx->nr_pstreams) {
             *cc_error = CC_INVALID_STREAM_ID_ERROR;
-- 
2.55.0
Re: [PATCH] hw/usb/hcd-xhci: Fix guest-triggerable assert() in xhci_find_stream()
Posted by Peter Maydell 1 week, 1 day ago
On Wed, 15 Jul 2026 at 21:34, Thomas Huth <thuth@redhat.com> wrote:
>
> From: Thomas Huth <thuth@redhat.com>
>
> The assert() statement in xhci_find_stream() can be triggered by
> the guest (see bug tickets #273, #3895 and #3988 on gitlab.com).
> Turn it into a qemu_log_mask() instead to fix this problem.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/273
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/usb/hcd-xhci.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index c7e050e38fd..ae8add4227e 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -1009,7 +1009,12 @@ static XHCIStreamContext *xhci_find_stream(XHCIEPContext *epctx,
>      dma_addr_t base;
>      uint32_t ctx[2], sct;
>
> -    assert(streamid != 0);
> +    if (!streamid) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "xhci: stream ID is zero\n");
> +        *cc_error = CC_INVALID_STREAM_ID_ERROR;
> +        return NULL;
> +    }
> +

This matches the error code that we should use for this case
of zero stream ID when checking the stream IDs in USB packets,
as per the XHCI spec 4.12.2.1 "Stream Array Bounds Checking",
which seems to be what the other error codes in the function
follow. (I'm looking at
https://www.intel.com/content/dam/www/public/us/en/documents/technical-specifications/extensible-host-controler-interface-usb-xhci.pdf
as the spec here.)

However, we don't actually seem to use this function for that
purpose. We call it in three places:
 - xhci_set_ep_dequeue(), which is the handler for the
   "Set TR Dequeue Pointer Command"
 - xhci_stall_ep, which ignores the error code
 - xhci_kick_epctx, which also ignores the error code

For the "Set TR Dequeue Pointer" case, the spec states that
the right error code for all cases of "bogus stream ID" is
"TRB Error", including "max_pstreams > 0 and stream ID == 0
(see 4.12.2.1 and 4.6.10.)

So:
(1) as far as this patch goes,
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

(2) xhci_set_ep_dequeue() ought to be doing

        sctx = xhci_find_stream(epctx, streamid, &err);
        if (sctx == NULL) {
            return CC_TRB_ERROR;
        }
rather than "return err;"

(3) xhci_set_ep_dequeue() should reject (CC_TRB_ERROR) a
non-zero streamid in its "nr_pstreams == 0" case, rather than
ignoring the streamid value

(4) Somewhere presumably we ought to check the stream ID
in a USB packet and halt the endpoint if it's out of range,
i.e. somewhere should be calling xhci_find_stream() and not
ignoring the error code...

thanks
-- PMM