Linux clock framework development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Ernest Van Hoecke" <ernestvanhoecke@gmail.com>
Cc: robh@kernel.org, devicetree@vger.kernel.org,
	linux-clk@vger.kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH v2 6/7] of: property: fw_devlink: Follow clock nexus maps
Date: Thu, 10 Sep 2026 14:29:36 +0000	[thread overview]
Message-ID: <20260910142937.6600C1F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260910-fw-devlink-nexus-ready-v2-6-17ac82dbe226@toradex.com>

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

Pre-existing issues:
- [Critical] Stack buffer overflow in `of_parse_phandle_with_args_map` due to unchecked `list_size` against stack array bounds.
- [High] The function `of_parse_phandle_with_args_map` can infinite loop if a device tree contains cyclic `<list>-map` properties (e.g., a cyclic `clock-map`).
- [High] Out-of-bounds read of `*-map-mask` and `*-map-pass-thru` properties based on unchecked lengths.
--

commit 44ed61de7bd2d691043db2b6326c916f60e9f97b
Author: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>

of: property: fw_devlink: Follow clock nexus maps

This patch modifies fw_devlink to resolve clock properties using the
map-aware parser instead of a direct phandle reference. This allows
fw_devlink to correctly identify the clock provider when clock nexus nodes
are used, preventing indefinite deferral.

Link: https://lore.kernel.org/all/20260807-schneider-v7-2-rc1-eip201-upstream-v5-0-8785f9531cf0@bootlin.com/

> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index a9c51d639a2b5..48e05f7bae2fd 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -1422,7 +1422,7 @@ struct supplier_bindings {
>  	u8 fwlink_flags;
>  };
>  
> -DEFINE_SIMPLE_PROP(clocks, "clocks", "#clock-cells")
> +DEFINE_SIMPLE_NEXUS_PROP(clocks, "clocks", "clock")

[Severity: High]
This isn't a bug introduced by this patch, but by switching fw_devlink to use
the nexus parser for clocks, it will now aggressively call
of_parse_phandle_with_args_map() during early boot for every clock property.
This exposes a few pre-existing flaws in that function to malformed device
trees, which can now crash or hang the boot process.

For instance, can a cyclic clock-map property cause an infinite loop here?

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

Because there is no depth limit or cycle detection, a self-referential map
will cause the kernel to loop forever.

[Severity: Critical]
This is a pre-existing issue, but does this loop write past the end of the
initial_match_array buffer?

drivers/of/base.c:of_parse_phandle_with_args_map() {
[ ... ]
	__be32 initial_match_array[MAX_PHANDLE_ARGS];
[ ... ]
	/* Get the #<list>-cells property */
	cur = out_args->np;
	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]);
[ ... ]
}

Since list_size is read directly from the device tree without being clamped
to MAX_PHANDLE_ARGS, a large #clock-cells value will overflow the stack
buffer.

[Severity: High]
This is a pre-existing issue, but are the mask and pass array lengths verified
against the property bounds before access?

drivers/of/base.c:of_parse_phandle_with_args_map() {
[ ... ]
		/* Get the <list>-map-mask property (optional) */
		mask = of_get_property(cur, mask_name, NULL);
		if (!mask)
			mask = dummy_mask;
		/* Iterate through <list>-map property */
		match = 0;
		while (map_len > (list_size + 1) && !match) {
			/* Compare specifiers */
			match = 1;
			for (i = 0; i < list_size; i++, map_len--)
				match &= !((match_array[i] ^ *map++) & mask[i]);
[ ... ]
		/* Get the <list>-map-pass-thru property (optional) */
		pass = of_get_property(cur, pass_name, NULL);
[ ... ]
			if (i < list_size) {
				val &= ~pass[i];
				val |= cpu_to_be32(out_args->args[i]) & pass[i];
			}
[ ... ]
}

Because of_get_property() is called with a NULL length pointer, if the
clock-map-mask or clock-map-pass-thru properties exist but are smaller
than list_size elements, mask[i] and pass[i] will read out of bounds.

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

  reply	other threads:[~2026-09-10 14:29 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-10 14:16 [PATCH v2 0/7] of: property: Make fw_devlink follow GPIO, PWM and clock nexus maps Ernest Van Hoecke
2026-09-10 14:16 ` [PATCH v2 1/7] of: property: fw_devlink: Follow GPIO " Ernest Van Hoecke
2026-09-10 14:28   ` sashiko-bot
2026-09-11  7:51   ` Bartosz Golaszewski
2026-09-10 14:16 ` [PATCH v2 2/7] of: property: fw_devlink: Report unsupported supplier linking Ernest Van Hoecke
2026-09-10 14:28   ` sashiko-bot
2026-09-10 14:16 ` [PATCH v2 3/7] of: unittest: Test fw_devlink with GPIO nexus maps Ernest Van Hoecke
2026-09-11  7:51   ` Bartosz Golaszewski
2026-09-10 14:16 ` [PATCH v2 4/7] of: property: fw_devlink: Follow PWM " Ernest Van Hoecke
2026-09-10 14:31   ` sashiko-bot
2026-09-10 14:16 ` [PATCH v2 5/7] of: unittest: Test fw_devlink with " Ernest Van Hoecke
2026-09-10 14:16 ` [PATCH v2 6/7] of: property: fw_devlink: Follow clock " Ernest Van Hoecke
2026-09-10 14:29   ` sashiko-bot [this message]
2026-09-10 14:16 ` [PATCH v2 7/7] of: unittest: Test fw_devlink with " Ernest Van Hoecke

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260910142937.6600C1F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=ernestvanhoecke@gmail.com \
    --cc=linux-clk@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox