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

[Dmytro Prokopchuk1 <dmytro_prokopchuk1@xxxxxxxx>]

On 7/9/26 12: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.)
> 
> Jan

Yeah... In DT Spec the values 0 and 3 are valid, actually.
Here it's Xen's limitation in implementation. So, "unsupported" is the 
most accurate description.

Well, also I see that this function has many silent returns. And only 
one resulting error message at the end:

     printk("Device tree generation failed (%d).\n", ret);

Maybe just drop that message?