[v1] hw/usb/hcd-xhci: Turn guest-triggerable abort() into qemu_log_mask()

[PATCH] hw/usb/hcd-xhci: Turn guest-triggerable abort() into qemu_log_mask()

Posted by Thomas Huth 1 day, 9 hours ago

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.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3784
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..9ff50d0b7f9 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");
             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");
             return CC_RESOURCE_ERROR;
         }
     }
-- 
2.55.0

Re: [PATCH] hw/usb/hcd-xhci: Turn guest-triggerable abort() into qemu_log_mask()

Posted by Philippe Mathieu-Daudé 1 day, 9 hours ago

On 10/7/26 16:27, Thomas Huth 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.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3784
> 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..9ff50d0b7f9 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");

With trailing \n:

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

>               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");
>               return CC_RESOURCE_ERROR;
>           }
>       }

Re: [PATCH] hw/usb/hcd-xhci: Turn guest-triggerable abort() into qemu_log_mask()

Posted by Thomas Huth 1 day, 9 hours ago

On 10/07/2026 16.36, Philippe Mathieu-Daudé wrote:
> On 10/7/26 16:27, Thomas Huth 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.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3784
>> 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..9ff50d0b7f9 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");
> 
> With trailing \n:

D'oh! Thanks!

I just noticed that I also forgot to add a "Reported-by" line to the 
description. I'll send a v2...

> Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Thanks!

  Thomas