Re: [PATCH v7 02/10] 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:
- [High] Lock-free access to the global `of_aliases` pointer in `of_find_node_opts_by_path()` introduces a TOCTOU Use-After-Free race with dynamic overlay detachments.
--

commit e8d4ce869294e7247761b17acbbfb6ac98d74f68
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

of: update /aliases lookup on reconfig notifications

This commit registers a reconfig notifier to mirror /aliases property changes
into aliases_lookup. The notifier chain covers changesets and overlays, matching
/aliases by name and root parent, and updates runtime-allocated entries.

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

>     The reference makes the lock-free
>     of_node_get(of_aliases) in of_find_node_opts_by_path() safe: a reader
>     that observes a non-NULL pointer observes a live node, and on DETACH
>     the changeset holds its own references across the notifier so the put
>     here cannot be the final one while readers still walk the node.

[Severity: High]
Is this description accurate?

If a thread calls of_find_node_opts_by_path() and is preempted immediately after
reading the of_aliases pointer but before calling of_node_get(), it could be
delayed arbitrarily.

If an overlay detachment completes concurrently, it drops the notifier's
reference and fully destroys the overlay changeset. When the preempted thread
resumes, wouldn't it call of_node_get() on freed memory?

> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index f7aa14d90e501..23d9bb073d8b5 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c

[ ... ]

> +static int of_aliases_reconfig_notifier(struct notifier_block *nb,
> +					unsigned long action, void *arg)
> +{
> +	struct of_reconfig_data *rd = arg;
> +
> +	/* of_aliases may still be NULL when an overlay creates the node */
> +	if (!rd->dn || !of_node_is_aliases(rd->dn))
> +		return NOTIFY_DONE;
> +
> +	mutex_lock(&aliases_mutex);
> +	switch (action) {
> +	case OF_RECONFIG_ATTACH_NODE:
> +		if (!of_aliases)
> +			of_aliases = of_node_get(rd->dn);
> +		break;
> +	case OF_RECONFIG_DETACH_NODE:
> +		if (of_aliases == rd->dn) {
> +			of_aliases = NULL;
> +			of_aliases_forget_all();
> +			of_node_put(rd->dn);

[Severity: High]
Can this introduce a Use-After-Free race with dynamic overlay detachments?

This overlay detachment clears the global pointer and drops the reference.
As discussed above, a lock-free reader could be preempted in
of_find_node_opts_by_path() right before acquiring the reference:

    aliases = of_node_get(of_aliases);

If the reader is delayed until after the overlay changeset is destroyed,
the node memory can be freed. Will the preempted thread then dereference
freed memory when it finally calls of_node_get()?

> +		}
> +		break;
> +	case OF_RECONFIG_ADD_PROPERTY:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-nh-of-alias-overlay-v7-0-02754604805a@nexthop.ai?part=2