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

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

[Dmytro Prokopchuk1]

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@epam.com>
---
Changes in v5:
- fixed mistakes around dprintk()
Test CI pipeline:
https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/2664600678
---
 xen/common/device-tree/dom0less-build.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index eacfd93087..c054ea4e2f 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -341,6 +341,14 @@ 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_cells %u or size_cells %u\n",
+                address_cells, size_cells);
+        return -EINVAL;
+    }
+
     node_next = fdt_first_subnode(pfdt, nodeoff);
     while ( node_next > 0 )
     {
-- 
2.43.0

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

[Oleksii Kurochko]

On 7/9/26 2:31 PM, 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@epam.com>
> ---
> Changes in v5:
> - fixed mistakes around dprintk()
> Test CI pipeline:
> https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/2664600678
> ---
>   xen/common/device-tree/dom0less-build.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
> index eacfd93087..c054ea4e2f 100644
> --- a/xen/common/device-tree/dom0less-build.c
> +++ b/xen/common/device-tree/dom0less-build.c
> @@ -341,6 +341,14 @@ 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_cells %u or size_cells %u\n",
> +                address_cells, size_cells);
> +        return -EINVAL;
> +    }
> +
>       node_next = fdt_first_subnode(pfdt, nodeoff);
>       while ( node_next > 0 )
>       {

LGTM:
  Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii

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

[Orzel, Michal]

On 09-Jul-26 14:31, 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@epam.com>
> ---
> Changes in v5:
> - fixed mistakes around dprintk()
> Test CI pipeline:
> https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/2664600678
> ---
>  xen/common/device-tree/dom0less-build.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
> index eacfd93087..c054ea4e2f 100644
> --- a/xen/common/device-tree/dom0less-build.c
> +++ b/xen/common/device-tree/dom0less-build.c
> @@ -341,6 +341,14 @@ 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) )
Too many discussions yesterday... Today, I looked at the complete flow once
again and I'm sorry to say that my previous comment to move the check right at
the read side was incorrect. #address/size-cells define the number of cells for
their children and these don't need to have xen,reg. Even our passthrough
example (docs/misc/arm/passthrough.txt) defines a ethernet node with #size-cells
= <0> for its PHY child with a 1 cell reg but no xen,reg. Here, for our sanity
check, we only care that number of cells for xen,reg is either 1 or 2. 0 is a
legitimate value for nodes without xen,reg (clock nodes, SPI, GPIO, etc.).
Values above 2 i.e. 3 is PCI only which does not go through this path but still,
we should only care about nodes with xen,reg.

Therefore, please move this check at the top of handle_passthrough_prop() with a
comment below (the number of discussions about this subject is a clear
indication that a comment is needed):

/*
 * 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.
 */

~Michal

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

[Dmytro Prokopchuk1]

On 7/10/26 10:24, Orzel, Michal wrote:
>
>
> On 09-Jul-26 14:31, 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@epam.com>
>> ---
>> Changes in v5:
>> - fixed mistakes around dprintk()
>> Test CI pipeline:
>> https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/2664600678
>> ---
>>   xen/common/device-tree/dom0less-build.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
>> index eacfd93087..c054ea4e2f 100644
>> --- a/xen/common/device-tree/dom0less-build.c
>> +++ b/xen/common/device-tree/dom0less-build.c
>> @@ -341,6 +341,14 @@ 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) )
> Too many discussions yesterday... Today, I looked at the complete flow once
> again and I'm sorry to say that my previous comment to move the check right at
> the read side was incorrect. #address/size-cells define the number of cells for
> their children and these don't need to have xen,reg. Even our passthrough
> example (docs/misc/arm/passthrough.txt) defines a ethernet node with #size-cells
> = <0> for its PHY child with a 1 cell reg but no xen,reg. Here, for our sanity
> check, we only care that number of cells for xen,reg is either 1 or 2. 0 is a
> legitimate value for nodes without xen,reg (clock nodes, SPI, GPIO, etc.).
> Values above 2 i.e. 3 is PCI only which does not go through this path but still,
> we should only care about nodes with xen,reg.
>
> Therefore, please move this check at the top of handle_passthrough_prop() with a
> comment below (the number of discussions about this subject is a clear
> indication that a comment is needed):
>
> /*
>   * 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.
>   */
>
> ~Michal
>

Ack. I will do.

Thanks, Dmytro.