[PATCH] hw/intc/loongarch_dintc: Fix OOB access in DINT MMIO write handler

Song Gao posted 1 patch 3 weeks, 4 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260701065454.1976188-1-gaosong@loongson.cn
Maintainers: Song Gao <gaosong@loongson.cn>, Bibo Mao <maobibo@loongson.cn>, Jiaxun Yang <jiaxun.yang@flygoat.com>
hw/intc/loongarch_dintc.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
[PATCH] hw/intc/loongarch_dintc: Fix OOB access in DINT MMIO write handler
Posted by Song Gao 3 weeks, 4 days ago
Validate guest-controlled cpu_num before using it to index the cpu[] array
or pass to async_run_on_cpu(). Without this check, a malicious guest can
trigger a NULL pointer dereference in async_run_on_cpu() and an
out-of-bounds array access in qemu_set_irq(), causing host crash.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3616
Fixes: 0d148eaf5a3e ("hw/loongarch: Implement dintc set irq")
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Song Gao <gaosong@loongson.cn>
---
 hw/intc/loongarch_dintc.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/intc/loongarch_dintc.c b/hw/intc/loongarch_dintc.c
index c877a8003b..e40292887c 100644
--- a/hw/intc/loongarch_dintc.c
+++ b/hw/intc/loongarch_dintc.c
@@ -19,6 +19,7 @@
 #include "target/loongarch/cpu.h"
 #include "qemu/error-report.h"
 #include "system/hw_accel.h"
+#include "qemu/log.h"
 
 /* msg addr field */
 FIELD(MSG_ADDR, IRQ_NUM, 4, 8)
@@ -52,7 +53,19 @@ static void loongarch_dintc_mem_write(void *opaque, hwaddr addr,
     CPUState *cs;
 
     cpu_num = FIELD_EX64(msg_addr, MSG_ADDR, CPU_NUM);
+
+    /* Validate cpu_num against the configured number of CPUs */
+    if (cpu_num >= s->num_cpu) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "loongarch-dintc: invalid cpu number%d\n", cpu_num);
+        return;
+    }
     cs = cpu_by_arch_id(cpu_num);
+    if (!cs) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "loongarch-dintc: no CPU for arch_id %d\n", cpu_num);
+        return;
+    }
     irq_num = FIELD_EX64(msg_addr, MSG_ADDR, IRQ_NUM);
 
     async_run_on_cpu(cs, do_set_vcpu_dintc_irq,
-- 
2.47.3
Re: [PATCH] hw/intc/loongarch_dintc: Fix OOB access in DINT MMIO write handler
Posted by Bibo Mao 2 weeks, 3 days ago

On 2026/7/1 下午2:54, Song Gao wrote:
> Validate guest-controlled cpu_num before using it to index the cpu[] array
> or pass to async_run_on_cpu(). Without this check, a malicious guest can
> trigger a NULL pointer dereference in async_run_on_cpu() and an
> out-of-bounds array access in qemu_set_irq(), causing host crash.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3616
> Fixes: 0d148eaf5a3e ("hw/loongarch: Implement dintc set irq")
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Song Gao <gaosong@loongson.cn>
> ---
>   hw/intc/loongarch_dintc.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/hw/intc/loongarch_dintc.c b/hw/intc/loongarch_dintc.c
> index c877a8003b..e40292887c 100644
> --- a/hw/intc/loongarch_dintc.c
> +++ b/hw/intc/loongarch_dintc.c
> @@ -19,6 +19,7 @@
>   #include "target/loongarch/cpu.h"
>   #include "qemu/error-report.h"
>   #include "system/hw_accel.h"
> +#include "qemu/log.h"
>   
>   /* msg addr field */
>   FIELD(MSG_ADDR, IRQ_NUM, 4, 8)
> @@ -52,7 +53,19 @@ static void loongarch_dintc_mem_write(void *opaque, hwaddr addr,
>       CPUState *cs;
>   
>       cpu_num = FIELD_EX64(msg_addr, MSG_ADDR, CPU_NUM);
> +
> +    /* Validate cpu_num against the configured number of CPUs */
> +    if (cpu_num >= s->num_cpu) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "loongarch-dintc: invalid cpu number%d\n", cpu_num);
> +        return;
> +    }
Here cpu_num decoded from MSG_ADDR is physical CPU ID, I think that it 
is not meaningful to compare it with s->num_cpu.

>       cs = cpu_by_arch_id(cpu_num);
This is ok to use cpu_by_arch_id now, in later when CPU hotplug is 
support with dintc, there should be self CPU search logic like ipi irqchip.

Regards
Bibo Mao
> +    if (!cs) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "loongarch-dintc: no CPU for arch_id %d\n", cpu_num);
> +        return;
> +    }
>       irq_num = FIELD_EX64(msg_addr, MSG_ADDR, IRQ_NUM);
>   
>       async_run_on_cpu(cs, do_set_vcpu_dintc_irq,
> 


Re: [PATCH] hw/intc/loongarch_dintc: Fix OOB access in DINT MMIO write handler
Posted by Thomas Huth 2 weeks, 3 days ago
On 09/07/2026 11.08, Bibo Mao wrote:
> 
> 
> On 2026/7/1 下午2:54, Song Gao wrote:
>> Validate guest-controlled cpu_num before using it to index the cpu[] array
>> or pass to async_run_on_cpu(). Without this check, a malicious guest can
>> trigger a NULL pointer dereference in async_run_on_cpu() and an
>> out-of-bounds array access in qemu_set_irq(), causing host crash.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3616
>> Fixes: 0d148eaf5a3e ("hw/loongarch: Implement dintc set irq")
>> Reported-by: Thomas Huth <thuth@redhat.com>
>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>> ---
>>   hw/intc/loongarch_dintc.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/hw/intc/loongarch_dintc.c b/hw/intc/loongarch_dintc.c
>> index c877a8003b..e40292887c 100644
>> --- a/hw/intc/loongarch_dintc.c
>> +++ b/hw/intc/loongarch_dintc.c
>> @@ -19,6 +19,7 @@
>>   #include "target/loongarch/cpu.h"
>>   #include "qemu/error-report.h"
>>   #include "system/hw_accel.h"
>> +#include "qemu/log.h"
>>   /* msg addr field */
>>   FIELD(MSG_ADDR, IRQ_NUM, 4, 8)
>> @@ -52,7 +53,19 @@ static void loongarch_dintc_mem_write(void *opaque, 
>> hwaddr addr,
>>       CPUState *cs;
>>       cpu_num = FIELD_EX64(msg_addr, MSG_ADDR, CPU_NUM);
>> +
>> +    /* Validate cpu_num against the configured number of CPUs */
>> +    if (cpu_num >= s->num_cpu) {
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "loongarch-dintc: invalid cpu number%d\n", cpu_num);
>> +        return;
>> +    }
> Here cpu_num decoded from MSG_ADDR is physical CPU ID, I think that it is 
> not meaningful to compare it with s->num_cpu.

Later in this function, cpu_num is used to index into the s->cpu[] array:

     qemu_set_irq(s->cpu[cpu_num].parent_irq, 1);

and s->cpu is allocated like this in the realize function:

     s->num_cpu = id_list->len;
     s->cpu = g_new(DINTCCore, s->num_cpu);

So the new check here looks valid for me, at least right now. Or what else 
would you suggest here?

  Thomas


>>       cs = cpu_by_arch_id(cpu_num);
> This is ok to use cpu_by_arch_id now, in later when CPU hotplug is support 
> with dintc, there should be self CPU search logic like ipi irqchip.
> 
> Regards
> Bibo Mao
>> +    if (!cs) {
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "loongarch-dintc: no CPU for arch_id %d\n", cpu_num);
>> +        return;
>> +    }
>>       irq_num = FIELD_EX64(msg_addr, MSG_ADDR, IRQ_NUM);
>>       async_run_on_cpu(cs, do_set_vcpu_dintc_irq,
>>
> 


Re: [PATCH] hw/intc/loongarch_dintc: Fix OOB access in DINT MMIO write handler
Posted by Bibo Mao 2 weeks, 3 days ago

On 2026/7/9 下午5:33, Thomas Huth wrote:
> On 09/07/2026 11.08, Bibo Mao wrote:
>>
>>
>> On 2026/7/1 下午2:54, Song Gao wrote:
>>> Validate guest-controlled cpu_num before using it to index the cpu[] 
>>> array
>>> or pass to async_run_on_cpu(). Without this check, a malicious guest can
>>> trigger a NULL pointer dereference in async_run_on_cpu() and an
>>> out-of-bounds array access in qemu_set_irq(), causing host crash.
>>>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3616
>>> Fixes: 0d148eaf5a3e ("hw/loongarch: Implement dintc set irq")
>>> Reported-by: Thomas Huth <thuth@redhat.com>
>>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>>> ---
>>>   hw/intc/loongarch_dintc.c | 13 +++++++++++++
>>>   1 file changed, 13 insertions(+)
>>>
>>> diff --git a/hw/intc/loongarch_dintc.c b/hw/intc/loongarch_dintc.c
>>> index c877a8003b..e40292887c 100644
>>> --- a/hw/intc/loongarch_dintc.c
>>> +++ b/hw/intc/loongarch_dintc.c
>>> @@ -19,6 +19,7 @@
>>>   #include "target/loongarch/cpu.h"
>>>   #include "qemu/error-report.h"
>>>   #include "system/hw_accel.h"
>>> +#include "qemu/log.h"
>>>   /* msg addr field */
>>>   FIELD(MSG_ADDR, IRQ_NUM, 4, 8)
>>> @@ -52,7 +53,19 @@ static void loongarch_dintc_mem_write(void 
>>> *opaque, hwaddr addr,
>>>       CPUState *cs;
>>>       cpu_num = FIELD_EX64(msg_addr, MSG_ADDR, CPU_NUM);
>>> +
>>> +    /* Validate cpu_num against the configured number of CPUs */
>>> +    if (cpu_num >= s->num_cpu) {
>>> +        qemu_log_mask(LOG_GUEST_ERROR,
>>> +                      "loongarch-dintc: invalid cpu number%d\n", 
>>> cpu_num);
>>> +        return;
>>> +    }
>> Here cpu_num decoded from MSG_ADDR is physical CPU ID, I think that it 
>> is not meaningful to compare it with s->num_cpu.
> 
> Later in this function, cpu_num is used to index into the s->cpu[] array:
> 
>      qemu_set_irq(s->cpu[cpu_num].parent_irq, 1);
> 
> and s->cpu is allocated like this in the realize function:
> 
>      s->num_cpu = id_list->len;
>      s->cpu = g_new(DINTCCore, s->num_cpu);
> 
> So the new check here looks valid for me, at least right now. Or what 
> else would you suggest here?

I prefer separate function dintc_cpu_by_arch_id(), this piece of code is 
not tested.

+static int loongarch_dintc_cmp(const void *a, const void *b)
+{
+   DINTCCore *dintc_a = (DINTCCore *)a;
+   DINTCCore *dintc_b = (DINTCCore *)b;
+
+   return dintc_a->arch_id - dintc_a->arch_id;
+}
+
+static int dintc_cpu_by_arch_id(LoongArchDINTCState *s,
+                                int64_t arch_id, int *index, CPUState 
**pcs)
+{
+    DINTCCore dintc, *found;
+
+    dintc.arch_id = arch_id;
+    found = bsearch(&dintc, s->cpu, s->num_cpu, sizeof(DINTCCore),
+                    loongarch_dintc_cmp);
+    if (found && found->cpu) {
+        if (index) {
+            *index = found - s->cpu;
+        }
+
+        if (pcs) {
+            *pcs = found->cpu;
+        }
+
+        return MEMTX_OK;
+    }
+
+    return MEMTX_ERROR;
+}
+
  static void loongarch_dintc_mem_write(void *opaque, hwaddr addr,
                                       uint64_t val, unsigned size)
  {
-    int irq_num, cpu_num = 0;
+    int irq_num, cpu_num, ret;
      LoongArchDINTCState *s = LOONGARCH_DINTC(opaque);
      uint64_t msg_addr = addr + VIRT_DINTC_BASE;
      CPUState *cs;
+    uint32_t cpuid;

-    cpu_num = FIELD_EX64(msg_addr, MSG_ADDR, CPU_NUM);
-    cs = cpu_by_arch_id(cpu_num);
+    cpuid = FIELD_EX64(msg_addr, MSG_ADDR, CPU_NUM);
      irq_num = FIELD_EX64(msg_addr, MSG_ADDR, IRQ_NUM);
+    ret = dintc_cpu_by_arch_id(s, cpuid, &cpu_num, &cs);
+    if (ret != MEMTX_OK || cpu_num >= ipi->num_cpu) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "loongarch-dintc: no CPU for arch_id %d\n", cpuid);
+        return;
+    }

      async_run_on_cpu(cs, do_set_vcpu_dintc_irq,
                           RUN_ON_CPU_HOST_INT(irq_num));

Regards
Bibo Mao
> 
>   Thomas
> 
> 
>>>       cs = cpu_by_arch_id(cpu_num);
>> This is ok to use cpu_by_arch_id now, in later when CPU hotplug is 
>> support with dintc, there should be self CPU search logic like ipi 
>> irqchip.
>>
>> Regards
>> Bibo Mao
>>> +    if (!cs) {
>>> +        qemu_log_mask(LOG_GUEST_ERROR,
>>> +                      "loongarch-dintc: no CPU for arch_id %d\n", 
>>> cpu_num);
>>> +        return;
>>> +    }
>>>       irq_num = FIELD_EX64(msg_addr, MSG_ADDR, IRQ_NUM);
>>>       async_run_on_cpu(cs, do_set_vcpu_dintc_irq,
>>>
>>
> 


Re: [PATCH] hw/intc/loongarch_dintc: Fix OOB access in DINT MMIO write handler
Posted by Thomas Huth 3 weeks, 3 days ago
On 01/07/2026 08.54, Song Gao wrote:
> Validate guest-controlled cpu_num before using it to index the cpu[] array
> or pass to async_run_on_cpu(). Without this check, a malicious guest can
> trigger a NULL pointer dereference in async_run_on_cpu() and an
> out-of-bounds array access in qemu_set_irq(), causing host crash.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3616
> Fixes: 0d148eaf5a3e ("hw/loongarch: Implement dintc set irq")
> Reported-by: Thomas Huth <thuth@redhat.com>
I think that should rather be:

Reported-by: huntr bubble

since I merely made you aware of the bug report at gitlab.

  Thomas
Re: [PATCH] hw/intc/loongarch_dintc: Fix OOB access in DINT MMIO write handler
Posted by Philippe Mathieu-Daudé 3 weeks, 4 days ago
Hi,

On 1/7/26 08:54, Song Gao wrote:
> Validate guest-controlled cpu_num before using it to index the cpu[] array
> or pass to async_run_on_cpu(). Without this check, a malicious guest can
> trigger a NULL pointer dereference in async_run_on_cpu() and an
> out-of-bounds array access in qemu_set_irq(), causing host crash.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3616
> Fixes: 0d148eaf5a3e ("hw/loongarch: Implement dintc set irq")
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Song Gao <gaosong@loongson.cn>
> ---
>   hw/intc/loongarch_dintc.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/hw/intc/loongarch_dintc.c b/hw/intc/loongarch_dintc.c
> index c877a8003b..e40292887c 100644
> --- a/hw/intc/loongarch_dintc.c
> +++ b/hw/intc/loongarch_dintc.c
> @@ -19,6 +19,7 @@
>   #include "target/loongarch/cpu.h"
>   #include "qemu/error-report.h"
>   #include "system/hw_accel.h"
> +#include "qemu/log.h"
>   
>   /* msg addr field */
>   FIELD(MSG_ADDR, IRQ_NUM, 4, 8)
> @@ -52,7 +53,19 @@ static void loongarch_dintc_mem_write(void *opaque, hwaddr addr,
>       CPUState *cs;
>   
>       cpu_num = FIELD_EX64(msg_addr, MSG_ADDR, CPU_NUM);
> +
> +    /* Validate cpu_num against the configured number of CPUs */
> +    if (cpu_num >= s->num_cpu) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "loongarch-dintc: invalid cpu number%d\n", cpu_num);
> +        return;

Do you know how real hardware behave in this case?

> +    }
>       cs = cpu_by_arch_id(cpu_num);
> +    if (!cs) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "loongarch-dintc: no CPU for arch_id %d\n", cpu_num);
> +        return;
> +    }
>       irq_num = FIELD_EX64(msg_addr, MSG_ADDR, IRQ_NUM);
>   
>       async_run_on_cpu(cs, do_set_vcpu_dintc_irq,
Re: [PATCH] hw/intc/loongarch_dintc: Fix OOB access in DINT MMIO write handler
Posted by gaosong 3 weeks, 3 days ago
在 2026/7/1 下午3:43, Philippe Mathieu-Daudé 写道:
> Hi,
>
> On 1/7/26 08:54, Song Gao wrote:
>> Validate guest-controlled cpu_num before using it to index the cpu[] 
>> array
>> or pass to async_run_on_cpu(). Without this check, a malicious guest can
>> trigger a NULL pointer dereference in async_run_on_cpu() and an
>> out-of-bounds array access in qemu_set_irq(), causing host crash.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3616
>> Fixes: 0d148eaf5a3e ("hw/loongarch: Implement dintc set irq")
>> Reported-by: Thomas Huth <thuth@redhat.com>
>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>> ---
>>   hw/intc/loongarch_dintc.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/hw/intc/loongarch_dintc.c b/hw/intc/loongarch_dintc.c
>> index c877a8003b..e40292887c 100644
>> --- a/hw/intc/loongarch_dintc.c
>> +++ b/hw/intc/loongarch_dintc.c
>> @@ -19,6 +19,7 @@
>>   #include "target/loongarch/cpu.h"
>>   #include "qemu/error-report.h"
>>   #include "system/hw_accel.h"
>> +#include "qemu/log.h"
>>     /* msg addr field */
>>   FIELD(MSG_ADDR, IRQ_NUM, 4, 8)
>> @@ -52,7 +53,19 @@ static void loongarch_dintc_mem_write(void 
>> *opaque, hwaddr addr,
>>       CPUState *cs;
>>         cpu_num = FIELD_EX64(msg_addr, MSG_ADDR, CPU_NUM);
>> +
>> +    /* Validate cpu_num against the configured number of CPUs */
>> +    if (cpu_num >= s->num_cpu) {
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "loongarch-dintc: invalid cpu number%d\n", 
>> cpu_num);
>> +        return;
>
> Do you know how real hardware behave in this case?
>
I haven't been able to verify the exact physical hardware behavior for 
writes targeting a non-existent CPU.
However, I don't think this situation will actually happen,

but QEMU ASAN has reported a code risk, and I'm not sure if we need to 
address it. Could you offer some advice?

Thanks.
Song Gao
>> +    }
>>       cs = cpu_by_arch_id(cpu_num);
>> +    if (!cs) {
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "loongarch-dintc: no CPU for arch_id %d\n", 
>> cpu_num);
>> +        return;
>> +    }
>>       irq_num = FIELD_EX64(msg_addr, MSG_ADDR, IRQ_NUM);
>>         async_run_on_cpu(cs, do_set_vcpu_dintc_irq,


Re: [PATCH] hw/intc/loongarch_dintc: Fix OOB access in DINT MMIO write handler
Posted by Thomas Huth 3 weeks, 3 days ago
On 02/07/2026 03.50, gaosong wrote:
> 在 2026/7/1 下午3:43, Philippe Mathieu-Daudé 写道:
>> Hi,
>>
>> On 1/7/26 08:54, Song Gao wrote:
>>> Validate guest-controlled cpu_num before using it to index the cpu[] array
>>> or pass to async_run_on_cpu(). Without this check, a malicious guest can
>>> trigger a NULL pointer dereference in async_run_on_cpu() and an
>>> out-of-bounds array access in qemu_set_irq(), causing host crash.
>>>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3616
>>> Fixes: 0d148eaf5a3e ("hw/loongarch: Implement dintc set irq")
>>> Reported-by: Thomas Huth <thuth@redhat.com>
>>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>>> ---
>>>   hw/intc/loongarch_dintc.c | 13 +++++++++++++
>>>   1 file changed, 13 insertions(+)
>>>
>>> diff --git a/hw/intc/loongarch_dintc.c b/hw/intc/loongarch_dintc.c
>>> index c877a8003b..e40292887c 100644
>>> --- a/hw/intc/loongarch_dintc.c
>>> +++ b/hw/intc/loongarch_dintc.c
>>> @@ -19,6 +19,7 @@
>>>   #include "target/loongarch/cpu.h"
>>>   #include "qemu/error-report.h"
>>>   #include "system/hw_accel.h"
>>> +#include "qemu/log.h"
>>>     /* msg addr field */
>>>   FIELD(MSG_ADDR, IRQ_NUM, 4, 8)
>>> @@ -52,7 +53,19 @@ static void loongarch_dintc_mem_write(void *opaque, 
>>> hwaddr addr,
>>>       CPUState *cs;
>>>         cpu_num = FIELD_EX64(msg_addr, MSG_ADDR, CPU_NUM);
>>> +
>>> +    /* Validate cpu_num against the configured number of CPUs */
>>> +    if (cpu_num >= s->num_cpu) {
>>> +        qemu_log_mask(LOG_GUEST_ERROR,
>>> +                      "loongarch-dintc: invalid cpu number%d\n", cpu_num);
>>> +        return;
>>
>> Do you know how real hardware behave in this case?
>>
> I haven't been able to verify the exact physical hardware behavior for 
> writes targeting a non-existent CPU.
> However, I don't think this situation will actually happen,
> 
> but QEMU ASAN has reported a code risk, and I'm not sure if we need to 
> address it. Could you offer some advice?

I don't know, but I guess real hardware would ignore the write, too - if 
there is no CPU with the right ID listening to the write, it's unlikely that 
something happens.

So I think your patch is doing the right thing:

Reviewed-by: Thomas Huth <thuth@redhat.com>


>>> +    }
>>>       cs = cpu_by_arch_id(cpu_num);
>>> +    if (!cs) {
>>> +        qemu_log_mask(LOG_GUEST_ERROR,
>>> +                      "loongarch-dintc: no CPU for arch_id %d\n", cpu_num);
>>> +        return;
>>> +    }
>>>       irq_num = FIELD_EX64(msg_addr, MSG_ADDR, IRQ_NUM);
>>>         async_run_on_cpu(cs, do_set_vcpu_dintc_irq,
> 


Re: [PATCH] hw/intc/loongarch_dintc: Fix OOB access in DINT MMIO write handler
Posted by Philippe Mathieu-Daudé 3 weeks, 2 days ago
On 2/7/26 08:14, Thomas Huth wrote:
> On 02/07/2026 03.50, gaosong wrote:
>> 在 2026/7/1 下午3:43, Philippe Mathieu-Daudé 写道:
>>> Hi,
>>>
>>> On 1/7/26 08:54, Song Gao wrote:
>>>> Validate guest-controlled cpu_num before using it to index the cpu[] 
>>>> array
>>>> or pass to async_run_on_cpu(). Without this check, a malicious guest 
>>>> can
>>>> trigger a NULL pointer dereference in async_run_on_cpu() and an
>>>> out-of-bounds array access in qemu_set_irq(), causing host crash.
>>>>
>>>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3616
>>>> Fixes: 0d148eaf5a3e ("hw/loongarch: Implement dintc set irq")
>>>> Reported-by: Thomas Huth <thuth@redhat.com>
>>>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>>>> ---
>>>>   hw/intc/loongarch_dintc.c | 13 +++++++++++++
>>>>   1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/hw/intc/loongarch_dintc.c b/hw/intc/loongarch_dintc.c
>>>> index c877a8003b..e40292887c 100644
>>>> --- a/hw/intc/loongarch_dintc.c
>>>> +++ b/hw/intc/loongarch_dintc.c
>>>> @@ -19,6 +19,7 @@
>>>>   #include "target/loongarch/cpu.h"
>>>>   #include "qemu/error-report.h"
>>>>   #include "system/hw_accel.h"
>>>> +#include "qemu/log.h"
>>>>     /* msg addr field */
>>>>   FIELD(MSG_ADDR, IRQ_NUM, 4, 8)
>>>> @@ -52,7 +53,19 @@ static void loongarch_dintc_mem_write(void 
>>>> *opaque, hwaddr addr,
>>>>       CPUState *cs;
>>>>         cpu_num = FIELD_EX64(msg_addr, MSG_ADDR, CPU_NUM);
>>>> +
>>>> +    /* Validate cpu_num against the configured number of CPUs */
>>>> +    if (cpu_num >= s->num_cpu) {
>>>> +        qemu_log_mask(LOG_GUEST_ERROR,
>>>> +                      "loongarch-dintc: invalid cpu number%d\n", 
>>>> cpu_num);
>>>> +        return;
>>>
>>> Do you know how real hardware behave in this case?
>>>
>> I haven't been able to verify the exact physical hardware behavior for 
>> writes targeting a non-existent CPU.
>> However, I don't think this situation will actually happen,
>>
>> but QEMU ASAN has reported a code risk, and I'm not sure if we need to 
>> address it. Could you offer some advice?

I was just checking, better to not diverge and be faithful when we can.

> 
> I don't know, but I guess real hardware would ignore the write, too - if 
> there is no CPU with the right ID listening to the write, it's unlikely 
> that something happens.
> 
> So I think your patch is doing the right thing:
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Maybe not "the right" but "a plausible" one ;)

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