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 range of [1, 2] at the top of handle_passthrough_prop().
Any invalid cell size combination is safely rejected early with an error
message and return -EINVAL.

Furthermore, update handle_passthrough_prop() to use the sizeof(*cell)
instead of sizeof(uint32_t).

Fixes: 9ce974c47588 ("xen/arm: assign devices to boot domains")
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
---
Changes in v6:
- move cells check at the top of handle_passthrough_prop() with a comment
- reword commit message
- replace sizeof(uint32_t) with sizeof(*cell) (use expression instead of type)
Test CI pipeline:
https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/2667596761
---
 xen/common/device-tree/dom0less-build.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index eacfd93087..9513c1c837 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -152,10 +152,23 @@ static int __init handle_passthrough_prop(struct kernel_info *kinfo,
             return -ENOMEM;
     }
 
+    /*
+     * xen,reg holds flat host/guest physical addresses and sizes, so the
+     * inherited #address-cells/#size-cells must each be 1 or 2. This also
+     * guards the len division below against a zero or wrapped divisor.
+     */
+    if ( (address_cells < 1) || (address_cells > 2) ||
+         (size_cells < 1) || (size_cells > 2) )
+    {
+        printk(XENLOG_ERR "Invalid address_cells %u or size_cells %u\n",
+               address_cells, size_cells);
+        return -EINVAL;
+    }
+
     /* xen,reg specifies where to map the MMIO region */
     cell = (const __be32 *)xen_reg->data;
     len = fdt32_to_cpu(xen_reg->len) / ((address_cells * 2 + size_cells) *
-                                        sizeof(uint32_t));
+                                        sizeof(*cell));
 
     for ( i = 0; i < len; i++ )
     {
-- 
2.43.0

On 12-Jul-26 13:56, 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 range of [1, 2] at the top of handle_passthrough_prop().
> Any invalid cell size combination is safely rejected early with an error
> message and return -EINVAL.
> 
> Furthermore, update handle_passthrough_prop() to use the sizeof(*cell)
> instead of sizeof(uint32_t).
> 
> Fixes: 9ce974c47588 ("xen/arm: assign devices to boot domains")
> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

You dropped "for-4.22" subject prefix. Was that intentional (you had it until
now)?. If not, please provide pros/cons for taking it into 4.22 and don't forget
to CC Oleksii (doing so now). It is not a critical bug and it's been with us for
a while now. That said, the fix is very simple and unharmful.

~Michal

> ---
> Changes in v6:
> - move cells check at the top of handle_passthrough_prop() with a comment
> - reword commit message
> - replace sizeof(uint32_t) with sizeof(*cell) (use expression instead of type)
> Test CI pipeline:
> https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/2667596761
> ---
>  xen/common/device-tree/dom0less-build.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
> index eacfd93087..9513c1c837 100644
> --- a/xen/common/device-tree/dom0less-build.c
> +++ b/xen/common/device-tree/dom0less-build.c
> @@ -152,10 +152,23 @@ static int __init handle_passthrough_prop(struct kernel_info *kinfo,
>              return -ENOMEM;
>      }
>  
> +    /*
> +     * xen,reg holds flat host/guest physical addresses and sizes, so the
> +     * inherited #address-cells/#size-cells must each be 1 or 2. This also
> +     * guards the len division below against a zero or wrapped divisor.
> +     */
> +    if ( (address_cells < 1) || (address_cells > 2) ||
> +         (size_cells < 1) || (size_cells > 2) )
> +    {
> +        printk(XENLOG_ERR "Invalid address_cells %u or size_cells %u\n",
> +               address_cells, size_cells);
> +        return -EINVAL;
> +    }
> +
>      /* xen,reg specifies where to map the MMIO region */
>      cell = (const __be32 *)xen_reg->data;
>      len = fdt32_to_cpu(xen_reg->len) / ((address_cells * 2 + size_cells) *
> -                                        sizeof(uint32_t));
> +                                        sizeof(*cell));
>  
>      for ( i = 0; i < len; i++ )
>      {

Hello Michal, Oleksii

Sorry, doing this v6... I just forgot to add "for-4.22".
Please, include this patch into 4.22.

BR, Dmytro.

On 7/13/26 10:21, Orzel, Michal wrote:
>
>
> On 12-Jul-26 13:56, 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 range of [1, 2] at the top of handle_passthrough_prop().
>> Any invalid cell size combination is safely rejected early with an error
>> message and return -EINVAL.
>>
>> Furthermore, update handle_passthrough_prop() to use the sizeof(*cell)
>> instead of sizeof(uint32_t).
>>
>> Fixes: 9ce974c47588 ("xen/arm: assign devices to boot domains")
>> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
>
> You dropped "for-4.22" subject prefix. Was that intentional (you had it until
> now)?. If not, please provide pros/cons for taking it into 4.22 and don't forget
> to CC Oleksii (doing so now). It is not a critical bug and it's been with us for
> a while now. That said, the fix is very simple and unharmful.
>
> ~Michal
>
>> ---
>> Changes in v6:
>> - move cells check at the top of handle_passthrough_prop() with a comment
>> - reword commit message
>> - replace sizeof(uint32_t) with sizeof(*cell) (use expression instead of type)
>> Test CI pipeline:
>> https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/2667596761
>> ---
>>   xen/common/device-tree/dom0less-build.c | 15 ++++++++++++++-
>>   1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
>> index eacfd93087..9513c1c837 100644
>> --- a/xen/common/device-tree/dom0less-build.c
>> +++ b/xen/common/device-tree/dom0less-build.c
>> @@ -152,10 +152,23 @@ static int __init handle_passthrough_prop(struct kernel_info *kinfo,
>>               return -ENOMEM;
>>       }
>>
>> +    /*
>> +     * xen,reg holds flat host/guest physical addresses and sizes, so the
>> +     * inherited #address-cells/#size-cells must each be 1 or 2. This also
>> +     * guards the len division below against a zero or wrapped divisor.
>> +     */
>> +    if ( (address_cells < 1) || (address_cells > 2) ||
>> +         (size_cells < 1) || (size_cells > 2) )
>> +    {
>> +        printk(XENLOG_ERR "Invalid address_cells %u or size_cells %u\n",
>> +               address_cells, size_cells);
>> +        return -EINVAL;
>> +    }
>> +
>>       /* xen,reg specifies where to map the MMIO region */
>>       cell = (const __be32 *)xen_reg->data;
>>       len = fdt32_to_cpu(xen_reg->len) / ((address_cells * 2 + size_cells) *
>> -                                        sizeof(uint32_t));
>> +                                        sizeof(*cell));
>>
>>       for ( i = 0; i < len; i++ )
>>       {
>

Hello Dmytro, Michal,

On 7/13/26 11:44 AM, Dmytro Prokopchuk1 wrote:
> Hello Michal, Oleksii
> 
> Sorry, doing this v6... I just forgot to add "for-4.22".
> Please, include this patch into 4.22.

I think that the fix is pretty simple so we could consider to be in 4.22:
  Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii