Re: [PATCH v5 6/7] clk: Add support for clock nexus dt bindings

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

Pre-existing issues:
- [High] Missing length validation on `*-map-mask` and `*-map-pass-thru` arrays in `of_parse_phandle_with_args_map()` allows out-of-bounds memory reads.
- [High] Lack of cycle detection in `of_parse_phandle_with_args_map()` allows a malicious device tree to cause an infinite loop in the kernel.
--

--- Patch [6]: [PATCH v5 6/7] clk: Add support for clock nexus dt bindings ---
commit a3b2f5d6812aa21284326e875a1ad049deda57c7
Author: Miquel Raynal (Schneider Electric) <[email protected]>

clk: Add support for clock nexus dt bindings

This patch introduces support for nexus nodes in the clock subsystem. It
transitions the parsing logic from of_parse_phandle_with_args() to
of_parse_phandle_with_args_map() to support clock-map properties.

> diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
> index 303a0bb26e54a..5380d43b56a42 100644
> --- a/drivers/clk/clk-conf.c
> +++ b/drivers/clk/clk-conf.c
> @@ -25,8 +25,8 @@ static int __set_clk_parents(struct device_node *node, bool clk_supplier)
>  		       node);
>  
>  	for (index = 0; index < num_parents; index++) {
> -		rc = of_parse_phandle_with_args(node, "assigned-clock-parents",
> -					"#clock-cells",	index, &clkspec);
> +		rc = of_parse_phandle_with_args_map(node, "assigned-clock-parents",
> +						    "clock", index, &clkspec);

[Severity: High]
This isn't a bug introduced by this patch, but changing to
of_parse_phandle_with_args_map() exposes the clock subsystem to a
pre-existing issue. Does of_parse_phandle_with_args_map() properly
validate the lengths of the map-mask and map-pass-thru arrays?

Looking at of_parse_phandle_with_args_map() in drivers/of/base.c, it seems
there are loops that unconditionally access mask[i] and pass[i] up to
list_size:

			for (i = 0; i < list_size; i++, map_len--)
				match &= !((match_array[i] ^ *map++) & mask[i]);

and later:

			if (i < list_size) {
				val &= ~pass[i];
				val |= cpu_to_be32(out_args->args[i]) & pass[i];
			}

If a device tree node provides clock-map-mask or clock-map-pass-thru properties
that are smaller than the expected size dictated by #clock-cells (list_size),
can this cause an out-of-bounds memory read?

[Severity: High]
This is a pre-existing issue, but does of_parse_phandle_with_args_map()
prevent infinite loops when parsing maliciously or accidentally crafted
device tree overlays?

In drivers/of/base.c:of_parse_phandle_with_args_map(), the main loop replaces
the current node with the new mapped node without tracking visited nodes or
limiting the mapping depth:

	while (cur) {
		/* Get the <list>-map property */

If a cyclic nexus mapping exists (e.g. node A maps to node B, and node B maps
back to node A), could this loop run indefinitely and hang the kernel?

>  		if (rc < 0) {
>  			/* skip empty (null) phandles */
>  			if (rc == -ENOENT)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807-schneider-v7-2-rc1-eip201-upstream-v5-0-8785f9531cf0@bootlin.com?part=6