hw/usb/hcd-xhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
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.
Reported-by: Feifan Qian <bea1e@proton.me>
Signed-off-by: Feifan Qian <bea1e@proton.me>
---
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
Hi
On Fri, Apr 24, 2026 at 4:37 PM Feifan Qian <bea1e@proton.me> 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.
>
> Reported-by: Feifan Qian <bea1e@proton.me>
> Signed-off-by: Feifan Qian <bea1e@proton.me>
> ---
> 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
>
--
Marc-André Lureau
© 2016 - 2026 Red Hat, Inc.