Re: [PATCH v2] of/address: Fix NULL bus dereference in of_pci_range_parser_one()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing upper bound validation for `#size-cells` (`ns`) allows `np` to overflow to a negative value, bypassing range bounds checks and causing an infinite out-of-bounds read loop.
--

commit 36a316bab0beafdbab73667fca4e9ad8ca56a5a3
Author: Carlo Caione <ccaione@baylibre.com>

of/address: Fix NULL bus dereference in of_pci_range_parser_one()

This commit rejects nodes with ranges/dma-ranges but no local
#address-cells in parser_init() to fix a NULL bus dereference. It also
keeps the DMA limit walk guarded by a non-empty dma-ranges property.

> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index cf4aab11e9b1f..fd2468b89579f 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -761,12 +762,20 @@ static int parser_init(struct of_pci_range_parser *parser,

[ ... ]

> -	parser->range = of_get_property(node, name, &rlen);
> -	if (parser->range == NULL)
> +	range = of_get_property(node, name, &rlen);
> +	if (!range)
>  		return -ENOENT;
>  
> -	parser->end = parser->range + rlen / sizeof(__be32);
> +	if (!parser->bus ||
> +	    !OF_CHECK_COUNTS(parser->na, parser->ns) ||

[Severity: High]
This is a pre-existing issue, but does OF_CHECK_COUNTS() need to validate an
upper bound for 'ns' (#size-cells)?

Currently, OF_CHECK_COUNTS() in drivers/of/of_private.h only validates that
'ns' is greater than 0, but unlike 'na', it does not enforce an upper limit:

#define OF_CHECK_COUNTS(na, ns) (OF_CHECK_ADDR_COUNT(na) && (ns) > 0)

If a malformed device tree provides a very large '#size-cells' value
(e.g., 0x7FFFFFFF), could this cause 'np' to overflow to a negative value in
of_pci_range_parser_one()?

drivers/of/address.c:of_pci_range_parser_one() {
    ...
    int np = parser->pna + na + ns;
    ...
    if (!parser->range || parser->range + np > parser->end)
        return NULL;
    ...
    size = of_read_number(parser->range + parser->pna + na, ns);

A negative 'np' would bypass the bounds check since subtracting from the pointer
evaluates to a smaller address. This would result in an out-of-bounds read
across memory when of_read_number() is called with the massive 'ns' value.

> +	    !OF_CHECK_ADDR_COUNT(parser->pna))
> +		return -EINVAL;
> +
> +	parser->range = range;
> +	parser->end = range + rlen / sizeof(__be32);
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706114731.57353-1-ccaione@baylibre.com?part=1