Re: [PATCH v4 1/6] of: resolve alias-prefixed paths under devtree_lock

[Krzysztof Kozlowski]

On Tue, Jul 21, 2026 at 06:01:46PM -0700, Abdurrahman Hussain wrote:
> of_find_node_opts_by_path() resolves an alias-prefixed path by
> walking the property list of the of_aliases node with no lock held
> and no reference taken on the node. Property surgery on /aliases —
> of_add_property(), of_remove_property(), changeset apply/revert —
> mutates that list under devtree_lock, so the lockless walk can step
> into a property that is being unlinked. Nothing keeps the node itself
> alive across the walk either: detaching /aliases and dropping the
> last reference frees the node and its property list mid-iteration.
> 
> The race has existed since the walk was introduced, but is hard to
> hit with a boot-FDT /aliases node that nothing ever detaches. The
> rest of this series makes /aliases dynamic — overlays can add and
> remove properties, and create and destroy the node itself — so close
> it first: find the matching property under devtree_lock with a
> reference held on of_aliases, and keep that reference across the
> of_find_node_by_path() call that resolves the value. The reference is
> what keeps the value string valid outside the lock: a concurrently
> removed property moves to the node's deadprops list and is only freed
> when the node itself is released.
> 
> While here, validate the value's shape before handing it to
> of_find_node_by_path(): /aliases contents can now come from overlays

Can it?

> and from raw changeset/of_add_property() callers, and only the
> overlay path validates at the producer. of_alias_value_ok() (shared
> with the alias tracking added later in this series) rejects values
> that are not non-empty C strings NUL-terminated within the property
> length, so the consumer no longer trusts any producer. This also
> covers the empty property (NULL value) that previously crashed in
> strchr().

All this looks like AI written.

> 
> The name match is folded into one bounded strncmp plus a check of the
> terminating NUL instead of strlen + strncmp, halving the string
> traversal now done with interrupts disabled.
> 
> Assisted-by: Claude:claude-fable-5 [Claude Code]
> Signed-off-by: Abdurrahman Hussain <abdurrahman@xxxxxxxxxx>
> ---
>  drivers/of/base.c       | 26 ++++++++++++++++++--------
>  drivers/of/of_private.h |  7 +++++++
>  2 files changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 6e7a42dedad3..9c2770823889 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -995,6 +995,8 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt
>  
>  	/* The path could begin with an alias */
>  	if (*path != '/') {
> +		struct device_node *aliases;
> +		const char *value = NULL;
>  		int len;
>  		const char *p = strchrnul(path, '/');
>  
> @@ -1002,16 +1004,24 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt
>  			p = separator;
>  		len = p - path;
>  
> -		/* of_aliases must not be NULL */
> -		if (!of_aliases)
> -			return NULL;
> -
> -		for_each_property_of_node(of_aliases, pp) {
> -			if (strlen(pp->name) == len && !strncmp(pp->name, path, len)) {
> -				np = of_find_node_by_path(pp->value);
> -				break;
> +		raw_spin_lock_irqsave(&devtree_lock, flags);

Since when do we lock looping over for_each_property_of_node()?

Do I understand correctly your proposal: you want to add
raw_spin_lock_irqsave() over every piece of code having for_each_XXX
OF-loop?

Best regards,
Krzysztof