imsic_init() indexes msi[] by the Xen CPU id hartid_to_cpuid() returns,
but that array is allocated with one entry per parent IRQ of the IMSIC
node, and the only range check compares the index against
num_possible_cpus(). Neither matches the array, and the check comes
after the first access:

- hartid_to_cpuid() returns NR_CPUS when the hart isn't one Xen brought
  up, and msi[NR_CPUS].base_addr is read before that is noticed;
- an IMSIC node listing fewer parents than Xen has CPUs makes every
  index past nr_parent_irqs go past the end of the allocation, which the
  num_possible_cpus() check lets through.

Size the array by nr_cpu_ids, which is what it is indexed by,
and move the range check ahead of the first msi[] access.

Fixes: c9bd8b322ecb ("xen/riscv: imsic_init() implementation")
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v2:
 - Use nr_cpu_ids instead of num_possible_cpus() to not be dependent on
   if cpu_possible_map is sparsed or not.
---
---
 xen/arch/riscv/imsic.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/xen/arch/riscv/imsic.c b/xen/arch/riscv/imsic.c
index f7b70a8da09e..44eb7abf76fd 100644
--- a/xen/arch/riscv/imsic.c
+++ b/xen/arch/riscv/imsic.c
@@ -326,6 +326,8 @@ int __init imsic_init(const struct dt_device_node *node)
     unsigned int nr_parent_irqs, index, nr_handlers = 0;
     paddr_t base_addr;
     unsigned int nr_mmios;
+    /* imsic_cfg.msi[] is indexed by Xen CPU id, so size it accordingly. */
+    unsigned int nr_msi = nr_cpu_ids;
     struct imsic_mmios *mmios;
     struct imsic_msi *msi = NULL;
 
@@ -346,7 +348,7 @@ int __init imsic_init(const struct dt_device_node *node)
         goto imsic_init_err;
     }
 
-    msi = xvzalloc_array(struct imsic_msi, nr_parent_irqs);
+    msi = xvzalloc_array(struct imsic_msi, nr_msi);
     if ( !msi )
     {
         rc = -ENOMEM;
@@ -405,7 +407,18 @@ int __init imsic_init(const struct dt_device_node *node)
             continue;
         }
 
+        /*
+         * hartid_to_cpuid() returns NR_CPUS for a hart Xen doesn't know, so
+         * the range has to be checked before msi[] is indexed at all.
+         */
         cpu = hartid_to_cpuid(hartid);
+        if ( cpu >= nr_msi )
+        {
+            printk(XENLOG_WARNING
+                   "%s: unsupported hart ID=%#lx for parent irq%u\n",
+                   node->name, hartid, i);
+            continue;
+        }
 
         /*
          * If .base_addr is not 0, it indicates that the CPU has already been
@@ -421,13 +434,6 @@ int __init imsic_init(const struct dt_device_node *node)
             continue;
         }
 
-        if ( cpu >= num_possible_cpus() )
-        {
-            printk(XENLOG_WARNING "%s: unsupported hart ID=%#lx for parent irq%u\n",
-                   node->name, hartid, i);
-            continue;
-        }
-
         /* Find MMIO location of MSI page */
         reloff = i * IMSIC_HART_SIZE(imsic_cfg.guest_index_bits);
         for ( index = 0; index < nr_mmios; index++ )
-- 
2.55.0

On 02.09.2026 16:16, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/imsic.c
> +++ b/xen/arch/riscv/imsic.c
> @@ -326,6 +326,8 @@ int __init imsic_init(const struct dt_device_node *node)
>      unsigned int nr_parent_irqs, index, nr_handlers = 0;
>      paddr_t base_addr;
>      unsigned int nr_mmios;
> +    /* imsic_cfg.msi[] is indexed by Xen CPU id, so size it accordingly. */
> +    unsigned int nr_msi = nr_cpu_ids;

Since there's no calculation involved anymore, is there a reason this
new variable is still needed? With it dropped, ...

> @@ -405,7 +407,18 @@ int __init imsic_init(const struct dt_device_node *node)
>              continue;
>          }
>  
> +        /*
> +         * hartid_to_cpuid() returns NR_CPUS for a hart Xen doesn't know, so
> +         * the range has to be checked before msi[] is indexed at all.
> +         */
>          cpu = hartid_to_cpuid(hartid);
> +        if ( cpu >= nr_msi )

... this comparison also will end up looking less odd.

Jan

On 9/2/26 4:49 PM, Jan Beulich wrote:
> On 02.09.2026 16:16, Oleksii Kurochko wrote:
>> --- a/xen/arch/riscv/imsic.c
>> +++ b/xen/arch/riscv/imsic.c
>> @@ -326,6 +326,8 @@ int __init imsic_init(const struct dt_device_node *node)
>>       unsigned int nr_parent_irqs, index, nr_handlers = 0;
>>       paddr_t base_addr;
>>       unsigned int nr_mmios;
>> +    /* imsic_cfg.msi[] is indexed by Xen CPU id, so size it accordingly. */
>> +    unsigned int nr_msi = nr_cpu_ids;
> 
> Since there's no calculation involved anymore, is there a reason this
> new variable is still needed? With it dropped, ...

I just thought that nr_msi will be more clear nr_cpu_ids but ...

> 
>> @@ -405,7 +407,18 @@ int __init imsic_init(const struct dt_device_node *node)
>>               continue;
>>           }
>>   
>> +        /*
>> +         * hartid_to_cpuid() returns NR_CPUS for a hart Xen doesn't know, so
>> +         * the range has to be checked before msi[] is indexed at all.
>> +         */
>>           cpu = hartid_to_cpuid(hartid);
>> +        if ( cpu >= nr_msi )
> 
> ... this comparison also will end up looking less odd.

... I was wrong. Will use nr_cpu_ids instead.

Thanks.

~ Oleksii