On Wed, 15 Jul 2026 at 21:34, Thomas Huth <[email protected]> wrote: > > From: Thomas Huth <[email protected]> > > 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 <[email protected]> > --- > 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 <[email protected]> (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
