Re: [PATCH v7 01/10] of: hold a reference on of_aliases during alias path resolution

["Abdurrahman Hussain" <abdurrahman@nexthop.ai>]

On Mon Aug 31, 2026 at 6:55 PM PDT, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Lockless load of the global `of_aliases` pointer followed by `of_node_get()` creates a Time-Of-Check to Time-Of-Use (TOCTOU) race condition that can lead to a Use-After-Free.
> --
>
> commit 6d7d618928c35a7237faefa2eebe21725f461ed9
> Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>
>
> of: hold a reference on of_aliases during alias path resolution
>     
> This commit updates of_find_node_opts_by_path() to hold a reference on the
> of_aliases node during alias path resolution. It also adds validation for
> alias values, checking that they are non-empty, NUL-terminated absolute paths.
>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index 378703dbc11f3..f7aa14d90e501 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
> [ ... ]
>> @@ -1002,16 +1004,21 @@ 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)
>> +		aliases = of_node_get(of_aliases);
>
> [Severity: High]
> Does this lockless load of the global of_aliases pointer followed by
> of_node_get() create a Time-Of-Check to Time-Of-Use (TOCTOU) race condition?
>
> If a reader thread evaluating of_aliases in of_find_node_opts_by_path() is
> preempted immediately after loading the pointer into a register, but before
> executing kobject_get() inside of_node_get(), a concurrent writer thread
> could completely remove the overlay containing the aliases node.
>
> The writer thread would drop all references and immediately free the node.
> When the reader thread resumes, it would pass the freed pointer to
> of_node_get(), which would dereference it to increment the refcount,
> resulting in a use-after-free.
>
>> +		if (!aliases)
>>  			return NULL;

In the strictest sense that window exists, but consider what it takes:
the reader must be preempted on the single instruction between the
pointer load and the refcount increment, and stay off-CPU while an
entire overlay teardown — changeset revert, notifiers, and
of_changeset_destroy(), all under of_mutex — starts and finishes. And
it only opens at all when /aliases itself is dynamically deleted,
i.e. the base DT booted without /aliases and the one overlay that
created it is reverted concurrently with an alias lookup. of_aliases
always holds a reference on the node it points to, the DETACH path
clears the pointer before dropping that reference, and the changeset
pins the node from the notifier until of_changeset_destroy() — so
there is no point at which a reader can load a pointer to an
already-freed node.

Closing the residual gap means serializing the reader against the
final of_node_put(). v6 did exactly that (devtree_lock around the
load + get) and Rob asked for the plain of_node_get() instead:

  https://lore.kernel.org/r/<rob-v6-reply-msgid>

The remaining options were already explored in this series' history:
keeping the DETACH reference (v3) trips __of_changeset_entry_destroy()'s
refcount check and leaks the node every apply/revert cycle, and putting
node lifetimes under RCU is a tree-wide change far beyond this series.
I'm keeping Rob's requested form.

Thanks,
Abdurrahman