Re: [PATCH] xen/device-tree: Fix off-by-one bounds check in make_memory_node()

[Oleksandr Tyshchenko <Oleksandr_Tyshchenko@xxxxxxxx>]

On 4/2/26 15:45, Orzel, Michal wrote:

Hello Michal

> 
> 
> On 02/04/2026 13:03, Oleksandr Tyshchenko wrote:
>> When building Xen with CONFIG_STATIC_SHM=n, booting a hardware
>> domain with exactly NR_MEM_BANKS (256) reserved-memory regions
>> causes a panic:
>>
>> (XEN) Xen BUG at common/device-tree/domain-build.c:497
>> (XEN) Xen call trace:
>> (XEN)    [<00000a0000289aa8>] make_memory_node+0x178/0x234 (PC)
>>
>> This occurs due to an off-by-one error in the bounds checking of
>> the reg array in make_memory_node(). The check:
>>      BUG_ON(nr_cells >= ARRAY_SIZE(reg));
>> incorrectly triggers when the array is exactly full (i.e., when
>> nr_cells == ARRAY_SIZE(reg)), preventing the 256th and final valid
>> memory region from being written.
>>
>> When CONFIG_STATIC_SHM=y, this bug remains hidden because
> AFAICT it remains hidden as long as you don't add NR_SHMEM_BANKS banks.
> In that case you will also hit this problem.

I think, you are right.

> 
>> DT_MEM_NODE_REG_RANGE_SIZE adds extra space for SHM banks.
>> This extra capacity prevents the array from ever reaching its
>> maximum limit while processing the 256th memory region.
>>
>> Fix this by changing the condition to strictly greater than (>).
>> Apply the exact same fix to shm_mem_node_fill_reg_range() to
>> prevent the same error.
>>
>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> This should have a Fixes tag.

It took some digging to locate the exact commit since this code has been 
moved and updated several times.

Initially, I thought about 9aaf437cd361 ("xen/arm: don't assign domU 
static-mem to dom0 as reserved-memory"). However, looking closely at the 
diff, that commit only moved the offending BUG_ON() inside a loop rather 
than introducing it. Unless I am mistaken, the correct target for the 
Fixes: is cd8015b634b0 ("ARM/dom0: Avoid using a variable length array 
in make_memory_node()").

As for shm_mem_node_fill_reg_range(), 7846f7699fea ("xen/arm: List 
static shared memory regions as /memory nodes") simply copied the 
existing BUG_ON() logic into the new function.


> With that:
> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

Thanks

> 
> ~Michal
>