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 num_possible_cpus(), 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>
---
 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..a32264518676 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 = num_possible_cpus();
     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 01.09.2026 17:50, 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 = num_possible_cpus();

The possible-CPUs-map may be sparse, so num_possible_cpus() may still yield
too small a value to use here. The thing to use likely is nr_cpu_ids. I notice
that variable is entirely unused so far by arch/riscv/*, though.

Jan

On 9/1/26 5:59 PM, Jan Beulich wrote:
> On 01.09.2026 17:50, 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 = num_possible_cpus();
> 
> The possible-CPUs-map may be sparse, so num_possible_cpus() may still yield
> too small a value to use here. The thing to use likely is nr_cpu_ids. I notice
> that variable is entirely unused so far by arch/riscv/*, though.
> 

IIUC "sparse" means we could have three CPUs with IDs 0, 1 and 12, so 
bits 0, 1 and 12 would be set in cpu_possible_map.

In that case num_possible_cpus() returns 3, which is the correct 
*number* of CPUs but msi[] is indexed by the Xen CPU ID, not by the 
CPU's ordinal position in the mask, so indexing it with ID 12 would need 
13 entries. The two only coincide when the mask is dense, so you are 
right that num_possible_cpus() is the wrong bound in principle.

That said, cpu_possible_map shouldn't be sparse by construction: the 
boot CPU is assigned ID 0 in smp_prepare_boot_cpu(), and the remaining 
IDs are handed out sequentially, so the bits are always 0..N-1 (for 
RISC-V it is done in such way in downstream). Arm, the other user of 
cpu_possible_map, fills it with Xen CPU IDs the same way.

I agree it is better not to depend on that, so I will use nr_cpu_ids for 
nr_msi instead.

~ Oleksii