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

[Abdurrahman Hussain]

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
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().

of_alias_value_ok() also requires the value to be an absolute path —
the DT spec defines alias values as full node paths. A relative value
that names another alias, including itself, would otherwise send
of_find_node_by_path() into unbounded mutual recursion with the alias
resolution here: `loop = "loop"` exhausts the kernel stack on the
first lookup. Both consumers (this reader and of_alias_create())
share the helper, so one check closes the recursion everywhere.

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 |  8 ++++++++
 2 files changed, 26 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);
+		aliases = of_node_get(of_aliases);
+		if (aliases) {
+			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;
+				}
 			}
 		}
+		raw_spin_unlock_irqrestore(&devtree_lock, flags);
+
+		/* the reference on @aliases keeps @value alive */
+		if (value)
+			np = of_find_node_by_path(value);
+		of_node_put(aliases);
 		if (!np)
 			return NULL;
 		path = p;
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 0ae16da066e2..9bba999f0bf8 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -215,6 +215,14 @@ static inline bool is_pseudo_property(const char *prop_name)
 		!of_prop_cmp(prop_name, "linux,phandle");
 }
 
+/* alias values must be absolute paths NUL-terminated within length */
+static inline bool of_alias_value_ok(const struct property *pp)
+{
+	return pp->value && pp->length >= 2 &&
+	       *(const char *)pp->value == '/' &&
+	       strnlen(pp->value, pp->length) < pp->length;
+}
+
 #if IS_ENABLED(CONFIG_KUNIT)
 int __of_address_resource_bounds(struct resource *r, u64 start, u64 size);
 #endif

-- 
2.54.0