From: Thomas Huth <thuth@redhat.com>

Some machines like the microvm machine instantiate a "sysbus-xhci"
device with just 1 interrupt (by setting the "intrs" property to 1).
xhci_sysbus_realize() then only allocates the s->irq array with one
entry.

Now the generic XHCI code has at least one code path that could call
the xhci_sysbus_intr_raise() function with n > 1, so this function
calls qemu_set_irq() with s->irq[n] pointing to a bad heap address.
The qemu_set_irq() then tries to call an IRQ handler via a function
pointer in that heap space - which either causes QEMU to die with
a segmentation fault (if it's a bad address), or even worse runs
some unexpected code if the destination of the pointer is executable
code.

Fix the problem by checking for valid interrupt numbers before
calling qemu_set_irq().

Fixes: CVE-2026-16043
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4001
Reported-by: Tristan Madani <tristan@talencesecurity.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/usb/hcd-xhci-sysbus.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci-sysbus.c b/hw/usb/hcd-xhci-sysbus.c
index 19664c5985e..7ab2f8527bd 100644
--- a/hw/usb/hcd-xhci-sysbus.c
+++ b/hw/usb/hcd-xhci-sysbus.c
@@ -8,6 +8,7 @@
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
 #include "qemu/osdep.h"
+#include "qemu/log.h"
 #include "hw/core/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "trace.h"
@@ -20,7 +21,12 @@ static bool xhci_sysbus_intr_raise(XHCIState *xhci, int n, bool level)
 {
     XHCISysbusState *s = container_of(xhci, XHCISysbusState, xhci);
 
-    qemu_set_irq(s->irq[n], level);
+    if (n < xhci->numintrs) {
+        qemu_set_irq(s->irq[n], level);
+    } else {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "unexpected IRQ #%i raised on xhci\n", n);
+    }
 
     return false;
 }
-- 
2.55.0

On Fri, 17 Jul 2026 at 12:30, Thomas Huth <thuth@redhat.com> wrote:
>
> From: Thomas Huth <thuth@redhat.com>
>
> Some machines like the microvm machine instantiate a "sysbus-xhci"
> device with just 1 interrupt (by setting the "intrs" property to 1).
> xhci_sysbus_realize() then only allocates the s->irq array with one
> entry.
>
> Now the generic XHCI code has at least one code path that could call
> the xhci_sysbus_intr_raise() function with n > 1, so this function
> calls qemu_set_irq() with s->irq[n] pointing to a bad heap address.
> The qemu_set_irq() then tries to call an IRQ handler via a function
> pointer in that heap space - which either causes QEMU to die with
> a segmentation fault (if it's a bad address), or even worse runs
> some unexpected code if the destination of the pointer is executable
> code.
>
> Fix the problem by checking for valid interrupt numbers before
> calling qemu_set_irq().
>
> Fixes: CVE-2026-16043
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4001
> Reported-by: Tristan Madani <tristan@talencesecurity.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/usb/hcd-xhci-sysbus.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/hw/usb/hcd-xhci-sysbus.c b/hw/usb/hcd-xhci-sysbus.c
> index 19664c5985e..7ab2f8527bd 100644
> --- a/hw/usb/hcd-xhci-sysbus.c
> +++ b/hw/usb/hcd-xhci-sysbus.c
> @@ -8,6 +8,7 @@
>   * SPDX-License-Identifier: GPL-2.0-or-later
>   */
>  #include "qemu/osdep.h"
> +#include "qemu/log.h"
>  #include "hw/core/qdev-properties.h"
>  #include "migration/vmstate.h"
>  #include "trace.h"
> @@ -20,7 +21,12 @@ static bool xhci_sysbus_intr_raise(XHCIState *xhci, int n, bool level)
>  {
>      XHCISysbusState *s = container_of(xhci, XHCISysbusState, xhci);
>
> -    qemu_set_irq(s->irq[n], level);
> +    if (n < xhci->numintrs) {
> +        qemu_set_irq(s->irq[n], level);
> +    } else {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "unexpected IRQ #%i raised on xhci\n", n);

This doesn't seem right. If we've told the core XHCI code that
we only have one interrupt line, then it should not be calling
this function for a nonexistent interrupt line.

In xhci_event() we have code that (a) has a special case
for numintrs == 1 that means all events go to interrupt 0,
and (b) have a bounds check for an out of range interrupt number.

It looks like the call path via the ERDP register write
is missing an equivalent check.

-- PMM

On 17/07/2026 15.02, Peter Maydell wrote:
> On Fri, 17 Jul 2026 at 12:30, Thomas Huth <thuth@redhat.com> wrote:
>>
>> From: Thomas Huth <thuth@redhat.com>
>>
>> Some machines like the microvm machine instantiate a "sysbus-xhci"
>> device with just 1 interrupt (by setting the "intrs" property to 1).
>> xhci_sysbus_realize() then only allocates the s->irq array with one
>> entry.
>>
>> Now the generic XHCI code has at least one code path that could call
>> the xhci_sysbus_intr_raise() function with n > 1, so this function
>> calls qemu_set_irq() with s->irq[n] pointing to a bad heap address.
>> The qemu_set_irq() then tries to call an IRQ handler via a function
>> pointer in that heap space - which either causes QEMU to die with
>> a segmentation fault (if it's a bad address), or even worse runs
>> some unexpected code if the destination of the pointer is executable
>> code.
>>
>> Fix the problem by checking for valid interrupt numbers before
>> calling qemu_set_irq().
>>
>> Fixes: CVE-2026-16043
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4001
>> Reported-by: Tristan Madani <tristan@talencesecurity.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   hw/usb/hcd-xhci-sysbus.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/usb/hcd-xhci-sysbus.c b/hw/usb/hcd-xhci-sysbus.c
>> index 19664c5985e..7ab2f8527bd 100644
>> --- a/hw/usb/hcd-xhci-sysbus.c
>> +++ b/hw/usb/hcd-xhci-sysbus.c
>> @@ -8,6 +8,7 @@
>>    * SPDX-License-Identifier: GPL-2.0-or-later
>>    */
>>   #include "qemu/osdep.h"
>> +#include "qemu/log.h"
>>   #include "hw/core/qdev-properties.h"
>>   #include "migration/vmstate.h"
>>   #include "trace.h"
>> @@ -20,7 +21,12 @@ static bool xhci_sysbus_intr_raise(XHCIState *xhci, int n, bool level)
>>   {
>>       XHCISysbusState *s = container_of(xhci, XHCISysbusState, xhci);
>>
>> -    qemu_set_irq(s->irq[n], level);
>> +    if (n < xhci->numintrs) {
>> +        qemu_set_irq(s->irq[n], level);
>> +    } else {
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "unexpected IRQ #%i raised on xhci\n", n);
> 
> This doesn't seem right. If we've told the core XHCI code that
> we only have one interrupt line, then it should not be calling
> this function for a nonexistent interrupt line.
> 
> In xhci_event() we have code that (a) has a special case
> for numintrs == 1 that means all events go to interrupt 0,
> and (b) have a bounds check for an out of range interrupt number.
> 
> It looks like the call path via the ERDP register write
> is missing an equivalent check.
Yes, the ERDP write is the critical code path. I can move the check to that 
part of the code, fine for me! I'll send a v2...

  Thomas

On Fri, 17 Jul 2026 at 14:13, Thomas Huth <thuth@redhat.com> wrote:
>
> On 17/07/2026 15.02, Peter Maydell wrote:
> >
> > This doesn't seem right. If we've told the core XHCI code that
> > we only have one interrupt line, then it should not be calling
> > this function for a nonexistent interrupt line.
> >
> > In xhci_event() we have code that (a) has a special case
> > for numintrs == 1 that means all events go to interrupt 0,
> > and (b) have a bounds check for an out of range interrupt number.
> >
> > It looks like the call path via the ERDP register write
> > is missing an equivalent check.
> Yes, the ERDP write is the critical code path. I can move the check to that
> part of the code, fine for me! I'll send a v2...

In particular, looking at the xhci spec, the registers
involved here are a set of registers for each interrupter,
and the spec says that it's up to the implementation how
many interrupters it has.

So I think that in xhci_runtime_read() and xhci_runtime_write()
where we do:
        int v = (reg - 0x20) / 0x20;
        XHCIInterrupter *intr = &xhci->intr[v];

we should be checking v against xhci->numintrs, and make
the access RAZ/WI if it's out of bounds. Compare

https://docs.amd.com/r/en-US/am011-versal-acap-trm/Port-Status-Control-Host-Interrupter-Event-Ring-and-Doorbell-Registers

which is an implementation of xhci with 4 interrupters: there are
4 registers of each type, not 16 (XHCI_MAXINTRS).

thanks
-- PMM

On 17/07/2026 15.48, Peter Maydell wrote:
> On Fri, 17 Jul 2026 at 14:13, Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 17/07/2026 15.02, Peter Maydell wrote:
>>>
>>> This doesn't seem right. If we've told the core XHCI code that
>>> we only have one interrupt line, then it should not be calling
>>> this function for a nonexistent interrupt line.
>>>
>>> In xhci_event() we have code that (a) has a special case
>>> for numintrs == 1 that means all events go to interrupt 0,
>>> and (b) have a bounds check for an out of range interrupt number.
>>>
>>> It looks like the call path via the ERDP register write
>>> is missing an equivalent check.
>> Yes, the ERDP write is the critical code path. I can move the check to that
>> part of the code, fine for me! I'll send a v2...
> 
> In particular, looking at the xhci spec, the registers
> involved here are a set of registers for each interrupter,
> and the spec says that it's up to the implementation how
> many interrupters it has.
> 
> So I think that in xhci_runtime_read() and xhci_runtime_write()
> where we do:
>          int v = (reg - 0x20) / 0x20;
>          XHCIInterrupter *intr = &xhci->intr[v];
> 
> we should be checking v against xhci->numintrs, and make
> the access RAZ/WI if it's out of bounds. Compare
> 
> https://docs.amd.com/r/en-US/am011-versal-acap-trm/Port-Status-Control-Host-Interrupter-Event-Ring-and-Doorbell-Registers
> 
> which is an implementation of xhci with 4 interrupters: there are
> 4 registers of each type, not 16 (XHCI_MAXINTRS).
Yes, I agree, I was also just looking into my local copy of the xHCI spec 
and I came to the same conclusion.

  Thomas