Re: [PATCH v5 2/9] of: update /aliases lookup on reconfig notifications

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Lockless iteration over `aliases->properties` in `of_find_node_opts_by_path()` races with concurrent property additions/removals.
--

--- Patch [2]: [PATCH v5 2/9] of: update /aliases lookup on reconfig notifications ---
commit 9797d916f1b83a48a93f588ed426470feb4bed4a
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

of: update /aliases lookup on reconfig notifications
    
Registers a reconfig notifier to mirror /aliases property changes into
aliases_lookup. The notifier chain covers changesets and overlays, matching
the /aliases node by name and root parent.

Link: https://lore.kernel.org/lkml/1435675876-2159-1-git-send-email-geert+renesas@glider.be/
Link: https://lore.kernel.org/lkml/20150630172131.D4E6CC4041A@trevor.secretlab.ca/

> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 6f79f593fc779..612e2cd31688d 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
[ ... ]
> +	case OF_RECONFIG_ADD_PROPERTY:
> +		of_alias_create(rd->prop, alias_alloc, true);
> +		break;
> +	case OF_RECONFIG_REMOVE_PROPERTY:
> +		of_alias_destroy(rd->prop->name);
> +		break;
> +	case OF_RECONFIG_UPDATE_PROPERTY:
> +		if (rd->old_prop)
> +			of_alias_destroy(rd->old_prop->name);
> +		of_alias_create(rd->prop, alias_alloc, true);
> +		break;

[Severity: High]
This isn't a bug introduced by this patch, but can this concurrent property
addition or removal race with the lockless property iteration in
of_find_node_opts_by_path()?

In drivers/of/base.c:of_find_node_opts_by_path(), aliases->properties is
iterated locklessly:

		for_each_property_of_node(aliases, pp) {
			if (!strncmp(pp->name, path, len) && !pp->name[len]) {
				if (of_alias_value_ok(pp))
					value = pp->value;
				break;
			}
		}

Concurrent property modifications via __of_add_property() or
__of_update_property() manipulate the list pointers without memory barriers.
A lockless reader may traverse to a newly added property before its contents
(like pp->name) are globally visible, leading to a crash. Additionally,
__of_update_property() repoints oldprop->next to deadprops, which can divert
a lockless reader into the dead properties list.

Could taking devtree_lock during the iteration, or utilizing the newly
synchronized aliases_lookup list, prevent this race?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-nh-of-alias-overlay-v5-0-2abe2bb9cdbc@nexthop.ai?part=2