Re: [PATCH v2 1/7] of: property: fw_devlink: Follow GPIO nexus maps

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Stack buffer overflow when parsing `#<stem>-cells` property.
- [High] Infinite loop during device tree map traversal.
--

commit 07389d4c4496496ba5a738fbae28d5c3df4427f8
Author: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>

of: property: fw_devlink: Follow GPIO nexus maps

This commit modifies fw_devlink to parse GPIO properties using a map-aware
parser instead of direct phandle references. This ensures fw_devlink resolves
the same provider as the GPIO consumer API, properly deferring consumers until
the nexus translation node is available.

Link: https://lore.kernel.org/all/juuc4af7ndbajcl7gzf4tg5qz2q2j5tt3rvql4jbauradujrre@gc5nbdhhixaf/

> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 72cf12907de03..380df0861ab97 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
[ ... ]
> @@ -1311,6 +1311,19 @@ static struct device_node *parse_prop_cells(struct device_node *np,
>  	return sup_args.np;
>  }
>  
> +static struct device_node *parse_nexus_prop(struct device_node *np,
> +					    const char *prop_name, int index,
> +					    const char *stem_name)
> +{
> +	struct of_phandle_args sup_args;
> +
> +	if (of_parse_phandle_with_args_map(np, prop_name, stem_name, index,
> +					   &sup_args))
> +		return NULL;
> +
> +	return sup_args.np;
> +}

[Severity: High]
This is a pre-existing issue, but by using of_parse_phandle_with_args_map()
during early boot for all -gpio(s) properties, does this expose a stack
buffer overflow to malformed Device Trees?

Looking at of_parse_phandle_with_args_map() in drivers/of/base.c, it reads
list_size from a Device Tree property (e.g., #gpio-cells) without checking
if it exceeds MAX_PHANDLE_ARGS:

drivers/of/base.c:of_parse_phandle_with_args_map() {
    ...
	ret = of_property_read_u32(cur, cells_name, &list_size);
	if (ret < 0)
		goto put;

	/* Precalculate the match array - this simplifies match loop */
	for (i = 0; i < list_size; i++)
		initial_match_array[i] = cpu_to_be32(out_args->args[i]);
    ...
}

If a malformed DT provides a cells property greater than 16, this loop will
read past the end of out_args->args and write past the end of the
initial_match_array stack buffer. Can this lead to memory corruption or a
kernel panic during early boot?


[Severity: High]
This isn't a bug introduced by this patch, but does calling
of_parse_phandle_with_args_map() here also expose an infinite loop when
traversing cyclic maps?

Looking at the map traversal loop in of_parse_phandle_with_args_map():

drivers/of/base.c:of_parse_phandle_with_args_map() {
    ...
	while (cur) {
		/* Get the <list>-map property */
		map = of_get_property(cur, map_name, &map_len);
    ...
		/* Iterate again with new provider */
		out_args->np = new;
		of_node_put(cur);
		cur = new;
		new = NULL;
	}
    ...
}

Since there is no depth limit or cycle detection, if a Device Tree has a
cyclic map structure (where Node A maps to Node B, which maps back to Node A),
will this loop hang infinitely and cause a hard lockup?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-fw-devlink-nexus-ready-v2-0-17ac82dbe226@toradex.com?part=1