On 24/04/2026 16.54, Marc-André Lureau wrote:
Hi
On Fri, Apr 24, 2026 at 4:37 PM Feifan Qian <[email protected]> wrote:
The xHCI endpoint context dword 0 bits 23:16 ("Interval") are written
by the guest and passed directly as the shift amount in:
epctx->interval = 1 << ((ctx[0] >> 16) & 0xff);
The shift amount can be 0-255. Shifting a 32-bit `int` left by >= 32
is undefined behaviour under C11 §6.5.7p4. With UBSan
(halt_on_error=1) this causes QEMU to abort; with aggressive compiler
optimisations that assume UB is unreachable the result is
unpredictable.
Clamp the exponent to [0, 31] with MIN() before the shift, and use
`1u` (unsigned) to avoid shifting a signed integer. The xHCI
specification defines a maximum meaningful Interval value of 15 for
most endpoint types; clamping to 31 is the minimal safe fix that
preserves the full unsigned 32-bit range for any compliant value.
I think we should clamp it to 15, to comply with the spec.
Otherwise, the patch looks fine.
Some endpoint types seem also to allow up to 18, so I'm going to pick up
this patch with 31 replaced by 18.
Thomas
Reported-by: Feifan Qian <[email protected]>
Signed-off-by: Feifan Qian <[email protected]>
---
hw/usb/hcd-xhci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 0000000000..0000000001 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1119,8 +1119,8 @@ static void xhci_init_epctx(XHCIEPContext *epctx,
xhci_ring_init(epctx->xhci, &epctx->ring, dequeue);
epctx->ring.ccs = ctx[2] & 1;
}
- epctx->interval = 1 << ((ctx[0] >> 16) & 0xff);
+ epctx->interval = 1u << MIN((ctx[0] >> 16) & 0xffu, 31u);
}
static TRBCCode xhci_enable_ep(XHCIState *xhci, unsigned int slotid,
--
2.43.0
