Re: [PATCH for-4.22 v3] dom0less: Prevent division by zero in handle_passthrough_prop()

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 09-Jul-26 11:50, Jan Beulich wrote:
> On 09.07.2026 11:44, Dmytro Prokopchuk1 wrote:
>> A malformed partial DTB specifying both '#address-cells = <0>' and
>> '#size-cells = <0>' causes '(address_cells * 2 + size_cells)' to
>> evaluate to 0. This sum is subsequently used as a divisor when
>> calculating the number of regions in the 'xen,reg' property inside
>> handle_passthrough_prop():
>>
>>     len = fdt32_to_cpu(xen_reg->len) / ((address_cells * 2 + size_cells) *
>>                                         sizeof(uint32_t));
>>
>> This leads to a division by zero exception in the Xen hypervisor during
>> boot, causing a hypervisor panic/crash.
>>
>> Fix this by validating that both 'address_cells' and 'size_cells'
>> are within the valid range of [1, 2] at the read side in scan_pfdt_node()
>> immediately after they are parsed. Any invalid cell size combination is
>> safely rejected early with an error message and return -EINVAL.
>>
>> Fixes: 9ce974c47588 ("xen/arm: assign devices to boot domains")
>> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@xxxxxxxx>
>> ---
>> Changes in v3:
>>  - use Michal's idea for placing that check into other place
>>  - reword commit message
>> ---
>>  xen/common/device-tree/dom0less-build.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/xen/common/device-tree/dom0less-build.c 
>> b/xen/common/device-tree/dom0less-build.c
>> index eacfd93087..179a2b88aa 100644
>> --- a/xen/common/device-tree/dom0less-build.c
>> +++ b/xen/common/device-tree/dom0less-build.c
>> @@ -341,6 +341,13 @@ static int __init scan_pfdt_node(struct kernel_info 
>> *kinfo, const void *pfdt,
>>      size_cells = device_tree_get_u32(pfdt, nodeoff, "#size-cells",
>>                                       DT_ROOT_NODE_SIZE_CELLS_DEFAULT);
>>  
>> +    if ( address_cells < 1 || address_cells > 2 ||
>> +         size_cells < 1 || size_cells > 2 )
>> +    {
>> +        dprintk(XENLOG_ERR "Invalid address/size cells combination\n");
> 
> Perhaps just for my own education: Is it really "invalid", or merely
> "unsupported" / "unrecognized"? (I can see 0 being possibly invalid, but
> it looks less clear for values above 2.)
AFAIR the DT spec does not mention the limit other than the type limit.
The libfdt defines limit as 4 (see `FDT_MAX_NCELLS` macro).
I've only seen #address-cells being 3 for PCI buses, where the first cell
defines the space type). For me, personally invalid vs unsupported is blurry
because if something is unsupported from Xen PoV, it is also invalid from Xen
PoV, so I tend not to comment on such details (though I'm not a native speaker).

@Dmytro, to prevent back and forth discussions:
 - add brackets
 - improve the message as Andrew suggested

~Michal