From: Thomas Huth <thuth@redhat.com>

The FIXME macros in xhci_alloc_device_streams() can be triggered
by a (malicious) guest. Since the macro also contains an abort()
statement, this terminates QEMU. Turn the FIXME statements into
a qemu_log_mask() instead to avoid that a guest can shoot itself
this way.

Reported-by: Feifan Qian <bea1e@proton.me>
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3784
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/usb/hcd-xhci.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 2cdab3ba0e4..002f801348f 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -965,11 +965,13 @@ static TRBCCode xhci_alloc_device_streams(XHCIState *xhci, unsigned int slotid,
          * together and make an usb_device_alloc_streams call per group.
          */
         if (epctxs[i]->nr_pstreams != req_nr_streams) {
-            FIXME("guest streams config not identical for all eps");
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "guest streams config not identical for all eps\n");
             return CC_RESOURCE_ERROR;
         }
         if (eps[i]->max_streams != dev_max_streams) {
-            FIXME("device streams config not identical for all eps");
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "device streams config not identical for all eps\n");
             return CC_RESOURCE_ERROR;
         }
     }
-- 
2.55.0

On Fri, 10 Jul 2026 at 15:44, Thomas Huth <thuth@redhat.com> wrote:
>
> From: Thomas Huth <thuth@redhat.com>
>
> The FIXME macros in xhci_alloc_device_streams() can be triggered
> by a (malicious) guest. Since the macro also contains an abort()
> statement, this terminates QEMU. Turn the FIXME statements into
> a qemu_log_mask() instead to avoid that a guest can shoot itself
> this way.
>
> Reported-by: Feifan Qian <bea1e@proton.me>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3784
> Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/usb/hcd-xhci.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index 2cdab3ba0e4..002f801348f 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -965,11 +965,13 @@ static TRBCCode xhci_alloc_device_streams(XHCIState *xhci, unsigned int slotid,
>           * together and make an usb_device_alloc_streams call per group.
>           */
>          if (epctxs[i]->nr_pstreams != req_nr_streams) {
> -            FIXME("guest streams config not identical for all eps");
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "guest streams config not identical for all eps\n");
>              return CC_RESOURCE_ERROR;
>          }
>          if (eps[i]->max_streams != dev_max_streams) {
> -            FIXME("device streams config not identical for all eps");
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "device streams config not identical for all eps\n");
>              return CC_RESOURCE_ERROR;

I get the impression from the comment that these cases are not
guest errors, but unimplemented QEMU support for a corner case
of the spec, so should be LOG_UNIMP. I had a quick look through
the XHCI spec, and it seems to define the streams handling
purely as a per-endpoint thing, making it valid for a device
to have multiple endpoints with different max streams settings.
It's just that it doesn't happen in practice so we haven't
implemented the "group identical endpoints and call
usb_device_alloc_streams for each group" solution the comment
sketches out.

Incidentally, there is just one remaining use of the FIXME macro
in this file: the "unhandled USB_RET_*" default case in
xhci_try_complete_packet(). I think we could (as a separate patch)
replace that with g_assert_not_reached(), because the function
handles all the USB_RET_* values except for USB_RET_ADD_TO_QUEUE and
USB_RET_REMOVE_FROM_QUEUE, which are both internal return
values for when an async packet needs to be queued or dequeued,
and which (I believe) shouldn't still be the status by the time we
get to this function.

There's also one raw printf that should be a LOG_UNIMP:
        fprintf(stderr, "xhci: FIXME: secondary streams not implemented yet");

thanks
-- PMM