[PATCH RFC 1/4] of: incrementally update /aliases lookup on reconfig notifications

[PATCH RFC 1/4] of: incrementally update /aliases lookup on reconfig notifications

[Abdurrahman Hussain]

/aliases entries added by a device-tree overlay are stored in the live
tree but never enter the global aliases_lookup list that of_alias_scan()
builds at boot. As a result, of_alias_get_id() returns -ENODEV for
aliases declared inside overlays, and any driver that relies on
alias-based numbering (i2c-xiic, spi, tty, mmc, ...) silently loses its
pinned id and falls back to auto-assignment.

Fix by registering an internal OF reconfig notifier from
of_core_init() that mirrors /aliases property changes into
aliases_lookup:

  OF_RECONFIG_ADD_PROPERTY    -> of_alias_create(pp, kzalloc, owned=true)
  OF_RECONFIG_REMOVE_PROPERTY -> of_alias_destroy(name)
  OF_RECONFIG_UPDATE_PROPERTY -> destroy + create

The reconfig notifier chain fires from both direct changesets and
overlay apply/revert, so the same code path covers runtime dt
modifications and overlay-declared aliases without any overlay-
specific hook in drivers/of/overlay.c. Grant Likely suggested this
shape on Geert Uytterhoeven's 2015 RFC [1]; Geert's original hook was
in dynamic.c directly.

Match the /aliases target node structurally (name == "aliases" and
parent == root) rather than by pointer against the of_aliases global.
A system with no boot-time /aliases has of_aliases == NULL, so an
overlay that creates /aliases from scratch would otherwise be missed
from the first ATTACH_NODE onward. The ATTACH/DETACH_NODE cases also
update of_aliases lazily so subsequent consumers see it.

Factor the per-property loop body of of_alias_scan() into
of_alias_create() so the boot-time scan and the runtime notifier share
one code path. Add a one-bit @owned flag to struct alias_prop tracking
whether the entry was kmalloc'd (overlay-time) or came from the boot-
time memblock allocator via of_alias_scan(). of_alias_destroy() skips
non-owned entries, so an overlay-driven UPDATE_PROPERTY against a
boot-time alias can't kfree() memblock storage — addressing the
allocator-mismatch worry Grant flagged on the 2015 series [2].

An @owned entry also holds an of_node_get() reference to its target,
released by of_alias_destroy(); this fixes a smaller leak Geert's
original of_alias_create() would have introduced for overlay targets.

Naming builds on Geert's original series:
  - "of: Extract of_alias_create()"                     [3]
  - "of: Add of_alias_destroy()"                        [4]
  - "of/dynamic: Update list of aliases on aliases changes" [5]

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]
Link: https://lore.kernel.org/lkml/1435675876-2159-2-git-send-email-geert+renesas@glider.be/ [3]
Link: https://lore.kernel.org/lkml/1435675876-2159-3-git-send-email-geert+renesas@glider.be/ [4]
Link: https://lore.kernel.org/lkml/1435675876-2159-4-git-send-email-geert+renesas@glider.be/ [5]
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/of/base.c       | 184 ++++++++++++++++++++++++++++++++++++++----------
 drivers/of/of_private.h |   7 ++
 2 files changed, 155 insertions(+), 36 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 6e7a42dedad3..d516523a71d5 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1915,6 +1915,152 @@ static void of_alias_add(struct alias_prop *ap, struct device_node *np,
 		 ap->alias, ap->stem, ap->id, np);
 }
 
+/*
+ * Build an alias_prop for @pp using @dt_alloc as the storage allocator
+ * and add it to aliases_lookup. @owned is stored on the entry so the
+ * matching destroy path knows whether the alias_prop is a kmalloc'd
+ * struct that must be kfree()d (with a paired of_node_put on the
+ * target) or a memblock/dt_alloc'd struct that must be left alone.
+ *
+ * Pseudo-properties (name, phandle, ...) and alias names not ending in
+ * a numeric id are silently skipped.
+ */
+static void of_alias_create(const struct property *pp,
+			    void *(*dt_alloc)(u64 size, u64 align),
+			    bool owned)
+{
+	const char *start = pp->name;
+	const char *end = start + strlen(start);
+	struct device_node *np;
+	struct alias_prop *ap;
+	int id, len;
+
+	if (is_pseudo_property(pp->name))
+		return;
+
+	np = of_find_node_by_path(pp->value);
+	if (!np)
+		return;
+
+	while (isdigit(*(end - 1)) && end > start)
+		end--;
+	len = end - start;
+
+	if (kstrtoint(end, 10, &id) < 0)
+		goto out_put;
+
+	ap = dt_alloc(sizeof(*ap) + len + 1, __alignof__(*ap));
+	if (!ap)
+		goto out_put;
+	memset(ap, 0, sizeof(*ap) + len + 1);
+	ap->alias = start;
+	ap->owned = owned;
+	of_alias_add(ap, np, id, start, len);
+	return;
+
+out_put:
+	/*
+	 * Boot-time entries reach here on parse failure; leaking the
+	 * of_node_get() from of_find_node_by_path() is fine because the
+	 * boot tree is never freed. Overlay-time entries need the put so
+	 * the target node's refcount tracks the failed-parse case
+	 * symmetrically with success.
+	 */
+	if (owned)
+		of_node_put(np);
+}
+
+/*
+ * Reverse of of_alias_create(): find the owned alias_prop whose name
+ * matches @name, unlink it, and release everything it owns. Non-owned
+ * (boot-time) entries are skipped so an overlay-driven UPDATE against
+ * a boot-time alias can't kfree() memblock storage. That does mean an
+ * overlay updating a boot-time alias leaves two entries in
+ * aliases_lookup — deferred as a separate cleanup.
+ */
+static void of_alias_destroy(const char *name)
+{
+	struct alias_prop *ap, *tmp;
+
+	list_for_each_entry_safe(ap, tmp, &aliases_lookup, link) {
+		if (!ap->owned || strcmp(ap->alias, name) != 0)
+			continue;
+		list_del(&ap->link);
+		of_node_put(ap->np);
+		kfree(ap);
+		return;
+	}
+}
+
+static void *alias_alloc(u64 size, u64 align)
+{
+	return kzalloc(size, GFP_KERNEL);
+}
+
+/*
+ * OF reconfig notifier that mirrors /aliases property changes into
+ * aliases_lookup. Fires on both direct changesets and overlay
+ * apply/revert, so of_alias_get_id() returns the right id for aliases
+ * declared inside an overlay.
+ */
+static int of_aliases_reconfig_notifier(struct notifier_block *nb,
+					unsigned long action, void *arg)
+{
+	struct of_reconfig_data *rd = arg;
+
+	/*
+	 * Match /aliases structurally (name + root-parent) rather than by
+	 * pointer against the of_aliases global — a system with no
+	 * boot-time /aliases (of_aliases == NULL) can still acquire one
+	 * from an overlay, and we must track its properties from the
+	 * first ATTACH_NODE onward.
+	 */
+	if (!rd->dn || !rd->dn->parent ||
+	    !of_node_is_root(rd->dn->parent) ||
+	    !of_node_name_eq(rd->dn, "aliases"))
+		return NOTIFY_DONE;
+
+	switch (action) {
+	case OF_RECONFIG_ATTACH_NODE:
+		if (!of_aliases)
+			of_aliases = rd->dn;
+		break;
+	case OF_RECONFIG_DETACH_NODE:
+		if (of_aliases == rd->dn)
+			of_aliases = NULL;
+		break;
+	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:
+		of_alias_destroy(rd->old_prop->name);
+		of_alias_create(rd->prop, alias_alloc, true);
+		break;
+	default:
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block of_aliases_nb = {
+	.notifier_call = of_aliases_reconfig_notifier,
+};
+
+static int __init of_aliases_reconfig_init(void)
+{
+	return of_reconfig_notifier_register(&of_aliases_nb);
+}
+
+/*
+ * of_alias_scan() runs from of_core_init() (core_initcall), so hook the
+ * reconfig notifier one initcall level later to guarantee the initial
+ * static scan is complete before any dynamic tracking begins.
+ */
+core_initcall_sync(of_aliases_reconfig_init);
+
 /**
  * of_alias_scan - Scan all properties of the 'aliases' node
  * @dt_alloc:	An allocator that provides a virtual address to memory
@@ -1950,42 +2096,8 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
 	if (!of_aliases)
 		return;
 
-	for_each_property_of_node(of_aliases, pp) {
-		const char *start = pp->name;
-		const char *end = start + strlen(start);
-		struct device_node *np;
-		struct alias_prop *ap;
-		int id, len;
-
-		/* Skip those we do not want to proceed */
-		if (is_pseudo_property(pp->name))
-			continue;
-
-		np = of_find_node_by_path(pp->value);
-		if (!np)
-			continue;
-
-		/* walk the alias backwards to extract the id and work out
-		 * the 'stem' string */
-		while (isdigit(*(end-1)) && end > start)
-			end--;
-		len = end - start;
-
-		if (kstrtoint(end, 10, &id) < 0) {
-			of_node_put(np);
-			continue;
-		}
-
-		/* Allocate an alias_prop with enough space for the stem */
-		ap = dt_alloc(sizeof(*ap) + len + 1, __alignof__(*ap));
-		if (!ap) {
-			of_node_put(np);
-			continue;
-		}
-		memset(ap, 0, sizeof(*ap) + len + 1);
-		ap->alias = start;
-		of_alias_add(ap, np, id, start, len);
-	}
+	for_each_property_of_node(of_aliases, pp)
+		of_alias_create(pp, dt_alloc, false);
 }
 
 /**
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 0ae16da066e2..9d16765ae2c3 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -17,6 +17,12 @@
  * @alias:	Alias property name
  * @np:		Pointer to device_node that the alias stands for
  * @id:		Index value from end of alias name
+ * @owned:	True if @alias was kstrdup'd and @np was of_node_get'd on
+ *		insertion (overlay-time entries). False for entries built
+ *		by of_alias_scan() at boot, where @alias points into the
+ *		FDT and @np is an unreferenced pointer. The removal path
+ *		uses this flag to decide whether it must kfree(@alias),
+ *		of_node_put(@np), and kfree(the struct itself).
  * @stem:	Alias string without the index
  *
  * The structure represents one alias property of 'aliases' node as
@@ -27,6 +33,7 @@ struct alias_prop {
 	const char *alias;
 	struct device_node *np;
 	int id;
+	bool owned;
 	char stem[];
 };
 

-- 
2.54.0

Re: [PATCH RFC 1/4] of: incrementally update /aliases lookup on reconfig notifications

[sashiko-bot]

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

New issues:
- [Critical] Unconditional dereference of `rd->old_prop` on OF_RECONFIG_UPDATE_PROPERTY causes a NULL pointer panic.
- [Critical] OF_RECONFIG_DETACH_NODE fails to remove dynamically allocated aliases, leading to a Memory Leak and Use-After-Free.
- [High] Lockless modification of the global `aliases_lookup` list causes data races and memory corruption.
- [High] Updating a boot-time alias creates duplicate entries mapping to the same ID, causing driver resource conflicts.
- [Medium] OF_RECONFIG_ATTACH_NODE fails to scan the node's properties, silently ignoring aliases added in a newly attached /aliases node.
- [Medium] The global `of_aliases` pointer mismanages node reference counting, causing leaks and weak references.
- [Low] The struct alias_prop::owned documentation contains factually incorrect and misleading claims regarding memory ownership.

Pre-existing issues:
- [Critical] Creating an alias from an empty property causes a NULL pointer dereference in of_find_node_by_path().
- [High] The alias parsing loop contains a 1-byte out-of-bounds read if the property name is an empty string.
--

--- Patch [1]: [PATCH RFC 1/4] of: incrementally update /aliases lookup on reconfig notifications ---
commit 172d1d81baebcd7bb4e961520fc11e6b4eadbc38
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

of: incrementally update /aliases lookup on reconfig notifications

This commit registers an OF reconfig notifier to mirror changes in the
/aliases node into the global aliases_lookup list. It extracts the alias
parsing logic into a common function and tracks whether entries are statically
or dynamically allocated. This ensures aliases defined in overlays are
correctly resolved by drivers.

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/
Link: https://lore.kernel.org/lkml/1435675876-2159-2-git-send-email-geert+renesas@glider.be/
Link: https://lore.kernel.org/lkml/1435675876-2159-3-git-send-email-geert+renesas@glider.be/
Link: https://lore.kernel.org/lkml/1435675876-2159-4-git-send-email-geert+renesas@glider.be/

> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 6e7a42dedad3f..d516523a71d5e 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c

[ ... ]

> @@ -1915,6 +1915,152 @@ static void of_alias_add(struct alias_prop *ap, struct device_node *np,
> +/*
> + * Build an alias_prop for @pp using @dt_alloc as the storage allocator
> + * and add it to aliases_lookup. @owned is stored on the entry so the
> + * matching destroy path knows whether the alias_prop is a kmalloc'd
> + * struct that must be kfree()d (with a paired of_node_put on the
> + * target) or a memblock/dt_alloc'd struct that must be left alone.
> + *
> + * Pseudo-properties (name, phandle, ...) and alias names not ending in
> + * a numeric id are silently skipped.
> + */
> +static void of_alias_create(const struct property *pp,
> +			    void *(*dt_alloc)(u64 size, u64 align),
> +			    bool owned)
> +{
> +	const char *start = pp->name;
> +	const char *end = start + strlen(start);
> +	struct device_node *np;
> +	struct alias_prop *ap;
> +	int id, len;
> +
> +	if (is_pseudo_property(pp->name))
> +		return;
> +
> +	np = of_find_node_by_path(pp->value);

[Severity: Critical]
This is a pre-existing issue, but does this cause a NULL pointer
dereference if the property is empty?

If an overlay or boot DT defines a property in /aliases with no value,
pp->value is NULL. Since of_alias_create() passes pp->value directly to
of_find_node_by_path(), which passes it to strchr(), will this crash?

> +	if (!np)
> +		return;
> +
> +	while (isdigit(*(end - 1)) && end > start)
> +		end--;

[Severity: High]
This isn't a bug introduced by this patch, but does the loop condition
cause an out-of-bounds read if the property name is an empty string?

If start is empty, end equals start. The loop evaluates isdigit(*(end - 1))
before verifying end > start, causing an immediate read of the memory byte
preceding the buffer.

> +	len = end - start;

[ ... ]

> +static void of_alias_destroy(const char *name)
> +{
> +	struct alias_prop *ap, *tmp;
> +
> +	list_for_each_entry_safe(ap, tmp, &aliases_lookup, link) {
> +		if (!ap->owned || strcmp(ap->alias, name) != 0)
> +			continue;

[Severity: High]
If an overlay modifies the target of an existing boot-time alias, will
skipping the destruction of boot-time entries leave duplicate entries in
the list?

Since this skips entries where owned is false, the old boot-time alias
remains in aliases_lookup alongside the newly created overlay alias.
When of_alias_get_id() searches the list, could it return the same ID for
both devices?

> +		list_del(&ap->link);
> +		of_node_put(ap->np);
> +		kfree(ap);
> +		return;
> +	}
> +}

[ ... ]

> +static int of_aliases_reconfig_notifier(struct notifier_block *nb,
> +					unsigned long action, void *arg)
> +{
> +	struct of_reconfig_data *rd = arg;
> +
> +	/*
> +	 * Match /aliases structurally (name + root-parent) rather than by
> +	 * pointer against the of_aliases global — a system with no
> +	 * boot-time /aliases (of_aliases == NULL) can still acquire one
> +	 * from an overlay, and we must track its properties from the
> +	 * first ATTACH_NODE onward.
> +	 */
> +	if (!rd->dn || !rd->dn->parent ||
> +	    !of_node_is_root(rd->dn->parent) ||
> +	    !of_node_name_eq(rd->dn, "aliases"))
> +		return NOTIFY_DONE;
> +
> +	switch (action) {
> +	case OF_RECONFIG_ATTACH_NODE:
> +		if (!of_aliases)
> +			of_aliases = rd->dn;

[Severity: Medium]
Does assigning of_aliases here create a weak reference?

During boot-time scanning, of_alias_scan() acquires of_aliases with an
incremented reference count. Here, it is assigned without calling
of_node_get().

[Severity: Medium]
Also, if the system started without a boot-time /aliases node, does
attaching one fail to parse its properties?

Since this handler updates the of_aliases pointer but misses the
for_each_property_of_node() loop to actually create the aliases, it seems
the aliases in a newly attached node are silently ignored.

> +		break;
> +	case OF_RECONFIG_DETACH_NODE:
> +		if (of_aliases == rd->dn)
> +			of_aliases = NULL;

[Severity: Medium]
Does clearing the pointer here cause a reference leak since of_node_put()
is omitted?

[Severity: Critical]
Additionally, does skipping the removal of the dynamic aliases cause a
memory leak and use-after-free?

The DETACH_NODE handler clears the of_aliases pointer but skips removing
the corresponding alias_prop entries from aliases_lookup. When the detached
node and its properties are later freed, dangling entries will remain in
the global list. A subsequent alias update could iterate the list and call
strcmp() on freed pointer strings.

> +		break;
> +	case OF_RECONFIG_ADD_PROPERTY:
> +		of_alias_create(rd->prop, alias_alloc, true);

[Severity: High]
Is it safe to modify aliases_lookup locklessly here?

The notifier processes dynamic changes and calls into of_alias_create()
which uses list_add_tail() on the global aliases_lookup list. OF reconfig
notifiers are intentionally executed after of_mutex has been unlocked, such
as in __of_changeset_apply_notify().

Could this race against other writers, or against driver probes calling
of_alias_get_id() which iterates the list expecting it to be protected by
of_mutex?

> +		break;
> +	case OF_RECONFIG_REMOVE_PROPERTY:
> +		of_alias_destroy(rd->prop->name);
> +		break;
> +	case OF_RECONFIG_UPDATE_PROPERTY:
> +		of_alias_destroy(rd->old_prop->name);

[Severity: Critical]
Can rd->old_prop be NULL here?

The of_update_property() API allows adding a property if it does not
already exist, and emits an OF_RECONFIG_UPDATE_PROPERTY event with
old_prop set to NULL. Would unconditionally dereferencing
rd->old_prop->name cause a panic in this scenario?

> +		of_alias_create(rd->prop, alias_alloc, true);
> +		break;
> +	default:
> +		break;
> +	}
> +	return NOTIFY_OK;
> +}

[ ... ]

> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index 0ae16da066e2c..9d16765ae2c3f 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -17,6 +17,12 @@
>   * @alias:	Alias property name
>   * @np:		Pointer to device_node that the alias stands for
>   * @id:		Index value from end of alias name
> + * @owned:	True if @alias was kstrdup'd and @np was of_node_get'd on
> + *		insertion (overlay-time entries). False for entries built
> + *		by of_alias_scan() at boot, where @alias points into the
> + *		FDT and @np is an unreferenced pointer. The removal path
> + *		uses this flag to decide whether it must kfree(@alias),
> + *		of_node_put(@np), and kfree(the struct itself).

[Severity: Low]
Does this documentation accurately reflect the implementation?

The comment claims @alias is kstrdup'd on insertion and kfree'd on
removal. However, the code merely stores the pp->name pointer directly
and never calls kfree() on the alias string. It also mentions np is
unreferenced at boot time, but of_find_node_by_path() does take a
reference.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-nh-of-alias-overlay-v1-0-f1e5d9889b30@nexthop.ai?part=1

Re: [PATCH RFC 1/4] of: incrementally update /aliases lookup on reconfig notifications

[Abdurrahman Hussain]

On Mon Jul 20, 2026 at 12:17 AM PDT, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
>
> New issues:
> - [Critical] Unconditional dereference of `rd->old_prop` on OF_RECONFIG_UPDATE_PROPERTY causes a NULL pointer panic.
> - [Critical] OF_RECONFIG_DETACH_NODE fails to remove dynamically allocated aliases, leading to a Memory Leak and Use-After-Free.
> - [High] Lockless modification of the global `aliases_lookup` list causes data races and memory corruption.
> - [High] Updating a boot-time alias creates duplicate entries mapping to the same ID, causing driver resource conflicts.
> - [Medium] OF_RECONFIG_ATTACH_NODE fails to scan the node's properties, silently ignoring aliases added in a newly attached /aliases node.
> - [Medium] The global `of_aliases` pointer mismanages node reference counting, causing leaks and weak references.
> - [Low] The struct alias_prop::owned documentation contains factually incorrect and misleading claims regarding memory ownership.
>
> Pre-existing issues:
> - [Critical] Creating an alias from an empty property causes a NULL pointer dereference in of_find_node_by_path().
> - [High] The alias parsing loop contains a 1-byte out-of-bounds read if the property name is an empty string.
> --
>
> --- Patch [1]: [PATCH RFC 1/4] of: incrementally update /aliases lookup on reconfig notifications ---
> commit 172d1d81baebcd7bb4e961520fc11e6b4eadbc38
> Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>
>
> of: incrementally update /aliases lookup on reconfig notifications
>
> This commit registers an OF reconfig notifier to mirror changes in the
> /aliases node into the global aliases_lookup list. It extracts the alias
> parsing logic into a common function and tracks whether entries are statically
> or dynamically allocated. This ensures aliases defined in overlays are
> correctly resolved by drivers.
>
> 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/
> Link: https://lore.kernel.org/lkml/1435675876-2159-2-git-send-email-geert+renesas@glider.be/
> Link: https://lore.kernel.org/lkml/1435675876-2159-3-git-send-email-geert+renesas@glider.be/
> Link: https://lore.kernel.org/lkml/1435675876-2159-4-git-send-email-geert+renesas@glider.be/
>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index 6e7a42dedad3f..d516523a71d5e 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>
> [ ... ]
>
>> @@ -1915,6 +1915,152 @@ static void of_alias_add(struct alias_prop *ap, struct device_node *np,
>> +/*
>> + * Build an alias_prop for @pp using @dt_alloc as the storage allocator
>> + * and add it to aliases_lookup. @owned is stored on the entry so the
>> + * matching destroy path knows whether the alias_prop is a kmalloc'd
>> + * struct that must be kfree()d (with a paired of_node_put on the
>> + * target) or a memblock/dt_alloc'd struct that must be left alone.
>> + *
>> + * Pseudo-properties (name, phandle, ...) and alias names not ending in
>> + * a numeric id are silently skipped.
>> + */
>> +static void of_alias_create(const struct property *pp,
>> +			    void *(*dt_alloc)(u64 size, u64 align),
>> +			    bool owned)
>> +{
>> +	const char *start = pp->name;
>> +	const char *end = start + strlen(start);
>> +	struct device_node *np;
>> +	struct alias_prop *ap;
>> +	int id, len;
>> +
>> +	if (is_pseudo_property(pp->name))
>> +		return;
>> +
>> +	np = of_find_node_by_path(pp->value);
>
> [Severity: Critical]
> This is a pre-existing issue, but does this cause a NULL pointer
> dereference if the property is empty?
>
> If an overlay or boot DT defines a property in /aliases with no value,
> pp->value is NULL. Since of_alias_create() passes pp->value directly to
> of_find_node_by_path(), which passes it to strchr(), will this crash?
>
>> +	if (!np)
>> +		return;
>> +
>> +	while (isdigit(*(end - 1)) && end > start)
>> +		end--;
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does the loop condition
> cause an out-of-bounds read if the property name is an empty string?
>
> If start is empty, end equals start. The loop evaluates isdigit(*(end - 1))
> before verifying end > start, causing an immediate read of the memory byte
> preceding the buffer.
>
>> +	len = end - start;
>
> [ ... ]
>
>> +static void of_alias_destroy(const char *name)
>> +{
>> +	struct alias_prop *ap, *tmp;
>> +
>> +	list_for_each_entry_safe(ap, tmp, &aliases_lookup, link) {
>> +		if (!ap->owned || strcmp(ap->alias, name) != 0)
>> +			continue;
>
> [Severity: High]
> If an overlay modifies the target of an existing boot-time alias, will
> skipping the destruction of boot-time entries leave duplicate entries in
> the list?
>
> Since this skips entries where owned is false, the old boot-time alias
> remains in aliases_lookup alongside the newly created overlay alias.
> When of_alias_get_id() searches the list, could it return the same ID for
> both devices?
>
>> +		list_del(&ap->link);
>> +		of_node_put(ap->np);
>> +		kfree(ap);
>> +		return;
>> +	}
>> +}
>
> [ ... ]
>
>> +static int of_aliases_reconfig_notifier(struct notifier_block *nb,
>> +					unsigned long action, void *arg)
>> +{
>> +	struct of_reconfig_data *rd = arg;
>> +
>> +	/*
>> +	 * Match /aliases structurally (name + root-parent) rather than by
>> +	 * pointer against the of_aliases global — a system with no
>> +	 * boot-time /aliases (of_aliases == NULL) can still acquire one
>> +	 * from an overlay, and we must track its properties from the
>> +	 * first ATTACH_NODE onward.
>> +	 */
>> +	if (!rd->dn || !rd->dn->parent ||
>> +	    !of_node_is_root(rd->dn->parent) ||
>> +	    !of_node_name_eq(rd->dn, "aliases"))
>> +		return NOTIFY_DONE;
>> +
>> +	switch (action) {
>> +	case OF_RECONFIG_ATTACH_NODE:
>> +		if (!of_aliases)
>> +			of_aliases = rd->dn;
>
> [Severity: Medium]
> Does assigning of_aliases here create a weak reference?
>
> During boot-time scanning, of_alias_scan() acquires of_aliases with an
> incremented reference count. Here, it is assigned without calling
> of_node_get().
>
> [Severity: Medium]
> Also, if the system started without a boot-time /aliases node, does
> attaching one fail to parse its properties?
>
> Since this handler updates the of_aliases pointer but misses the
> for_each_property_of_node() loop to actually create the aliases, it seems
> the aliases in a newly attached node are silently ignored.
>
>> +		break;
>> +	case OF_RECONFIG_DETACH_NODE:
>> +		if (of_aliases == rd->dn)
>> +			of_aliases = NULL;
>
> [Severity: Medium]
> Does clearing the pointer here cause a reference leak since of_node_put()
> is omitted?
>
> [Severity: Critical]
> Additionally, does skipping the removal of the dynamic aliases cause a
> memory leak and use-after-free?
>
> The DETACH_NODE handler clears the of_aliases pointer but skips removing
> the corresponding alias_prop entries from aliases_lookup. When the detached
> node and its properties are later freed, dangling entries will remain in
> the global list. A subsequent alias update could iterate the list and call
> strcmp() on freed pointer strings.
>
>> +		break;
>> +	case OF_RECONFIG_ADD_PROPERTY:
>> +		of_alias_create(rd->prop, alias_alloc, true);
>
> [Severity: High]
> Is it safe to modify aliases_lookup locklessly here?
>
> The notifier processes dynamic changes and calls into of_alias_create()
> which uses list_add_tail() on the global aliases_lookup list. OF reconfig
> notifiers are intentionally executed after of_mutex has been unlocked, such
> as in __of_changeset_apply_notify().
>
> Could this race against other writers, or against driver probes calling
> of_alias_get_id() which iterates the list expecting it to be protected by
> of_mutex?
>
>> +		break;
>> +	case OF_RECONFIG_REMOVE_PROPERTY:
>> +		of_alias_destroy(rd->prop->name);
>> +		break;
>> +	case OF_RECONFIG_UPDATE_PROPERTY:
>> +		of_alias_destroy(rd->old_prop->name);
>
> [Severity: Critical]
> Can rd->old_prop be NULL here?
>
> The of_update_property() API allows adding a property if it does not
> already exist, and emits an OF_RECONFIG_UPDATE_PROPERTY event with
> old_prop set to NULL. Would unconditionally dereferencing
> rd->old_prop->name cause a panic in this scenario?
>
>> +		of_alias_create(rd->prop, alias_alloc, true);
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +	return NOTIFY_OK;
>> +}
>
> [ ... ]
>
>> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
>> index 0ae16da066e2c..9d16765ae2c3f 100644
>> --- a/drivers/of/of_private.h
>> +++ b/drivers/of/of_private.h
>> @@ -17,6 +17,12 @@
>>   * @alias:	Alias property name
>>   * @np:		Pointer to device_node that the alias stands for
>>   * @id:		Index value from end of alias name
>> + * @owned:	True if @alias was kstrdup'd and @np was of_node_get'd on
>> + *		insertion (overlay-time entries). False for entries built
>> + *		by of_alias_scan() at boot, where @alias points into the
>> + *		FDT and @np is an unreferenced pointer. The removal path
>> + *		uses this flag to decide whether it must kfree(@alias),
>> + *		of_node_put(@np), and kfree(the struct itself).
>
> [Severity: Low]
> Does this documentation accurately reflect the implementation?
>
> The comment claims @alias is kstrdup'd on insertion and kfree'd on
> removal. However, the code merely stores the pp->name pointer directly
> and never calls kfree() on the alias string. It also mentions np is
> unreferenced at boot time, but of_find_node_by_path() does take a
> reference.

Good feedback! Will address in v2.

Thanks,
Abdurrahman

[PATCH RFC 0/4] of: teach overlay code to keep /aliases in sync

[Abdurrahman Hussain]

/aliases entries added by a device-tree overlay are stored in the live
tree but never enter the global aliases_lookup list that of_alias_scan()
builds at boot. As a result, of_alias_get_id() returns -ENODEV for
aliases declared inside overlays, and any driver that relies on
alias-based numbering (i2c-xiic, spi, tty, mmc, ...) silently loses its
pinned id and falls back to auto-assignment.

The gap has been public since 2015 [1] and reproduces trivially: apply
an overlay that declares e.g. `i2c99 = &foo;`, ask
of_alias_get_id(foo, "i2c") -> -ENODEV. Bootlin's ELCE 2025 talk on
PCI DT overlays [2] enumerates "i2c muxes" as one of the subsystems
broken by dynamic overlays; alias pinning is the underlying cause.

The core fix (patch 1) is a reconfig notifier that mirrors /aliases
property changes into aliases_lookup. Two smaller overlay-code fixes
(patches 2 and 3) fall out of the same use case: without them, the
notifier alone can't actually resolve overlay-declared aliases.

Prior art
---------

Geert Uytterhoeven posted a 3-patch RFC in June 2015 [1] with the same
alias-tracking design shape. Grant Likely reviewed positively; merge
was gated on missing unittests and an object-lifetime concern the
author self-flagged, and the series was never reposted as non-RFC. Ten
years later, drivers/of/overlay.c still contains zero references to
aliases, of_alias_scan, or aliases_lookup.

Series contents
---------------

Patch 1 adds a reconfig notifier that mirrors /aliases property
changes into aliases_lookup. It also refactors of_alias_scan()'s
per-property body into a helper of_alias_create() shared by the
boot-time scan and the runtime notifier. A one-bit `owned` flag on
struct alias_prop distinguishes kmalloc'd (overlay-time) entries
(kstrdup'd alias name, of_node_get'd target) from memblock-backed
(boot-time) ones so the remove path can't kfree the wrong storage.
The notifier keys off structural properties of the target node
(name + root-parent) rather than the of_aliases global, so overlays
that create /aliases from scratch on a system without a boot-time
aliases node are covered too. All aliases_lookup readers and writers
serialize on a dedicated mutex.

Patch 2 fixes find_target() so an overlay applied with a non-NULL
target base can still reach the DT root via target-path="/foo". The
current code unconditionally concatenates base + target-path via
"%pOF%s", so target-path="/aliases" resolves to "<base>/aliases" and
target-path="/" produces "<base>/" (never a valid node). After this
patch, an empty target-path continues to mean "the target base
itself" — preserving the shape used by drivers/misc/lan966x_pci.c,
the only in-tree caller of of_overlay_fdt_apply() that passes a
non-NULL base — while any non-empty target-path is looked up
absolutely.

Patch 3 rewrites /aliases property values from the overlay's internal
fragment path ("/fragment@N/__overlay__/...") to the live-tree path
that the node will occupy after apply. The overlay code already does
this for /__symbols__ via dup_and_fixup_symbol_prop(); /aliases uses
the same textual convention and can reuse the same helper. Without
this, patch 1's notifier receives paths that never resolve in the
live tree, so of_alias_get_id() still returns -ENODEV.

Patch 4 adds a unittest — overlay_alias.dtso plus
of_unittest_overlay_alias() — that applies an overlay with a
non-NULL target_base, declaring a labeled node under the base and an
`testcase-alias99 = &that-label` entry in DT-root /aliases, then
asserts of_alias_get_id() flips from -ENODEV to 99 across the apply,
and back to -ENODEV across the revert. Missing any one of the three
functional patches (notifier / absolute-target-paths / alias path
rewrite) causes the assertion to fail.

Changes in v2
-------------

Addresses the automated review of the RFC posting.

Patch 1:
  - Guard rd->old_prop before dereferencing in the UPDATE_PROPERTY
    handler; some notifier producers pass NULL.
  - Handle OF_RECONFIG_ATTACH_NODE and OF_RECONFIG_DETACH_NODE for
    /aliases nodes attached or detached with pre-populated properties;
    scan (or purge) each property so per-property ADD/REMOVE events
    aren't required.
  - Serialize aliases_lookup with a dedicated aliases_mutex; readers
    (of_alias_get_id, of_alias_get_highest_id) and the reconfig
    notifier both hold it. Boot-time of_alias_scan() stays lockless
    (single-threaded during init).
  - Destroy path unlinks matching entries irrespective of ownership
    (freeing storage only for owned ones), so an overlay UPDATE
    against a boot-time alias no longer leaves duplicate stem+id
    entries in aliases_lookup.
  - Owned entries kstrdup the alias name and hold an of_node_get()
    reference on the target — released in the destroy path — so a
    property freed by overlay revert can't dangle into aliases_lookup.
  - of_aliases is refcounted lazily on ATTACH/DETACH.
  - Validate pp->value is non-empty and null-terminated within
    pp->length before feeding it to of_find_node_by_path() — prevents
    a stray malformed alias entry from causing an OOB read.

Patch 3:
  - Only route /aliases properties through dup_and_fixup_symbol_prop()
    when the value looks like a fragment-internal path (non-empty,
    null-terminated, "/fragment@" prefix). Other alias values pass
    through the raw duplicator unchanged. This removes the previous
    unconditional fallback that would silently propagate a raw dup on
    ENOMEM, and also stops the notifier from ever seeing malformed
    values through this path.

Patch 4:
  - Rewrite the .dtso to actually exercise all three functional
    patches: a labeled node under a fragment with target-path="" plus
    an /aliases fragment with target-path="/aliases" (absolute) and
    a value referencing the label via `&`. The RFC test bypassed
    patches 2 and 3.
  - Rewrite the runner to call of_overlay_fdt_apply() directly with a
    non-NULL target_base, look up the grafted node by its post-apply
    live-tree path, and assert against it.

Open items
----------

None outstanding. Pre-existing issues surfaced by the automated review
(overlay-changeset leak paths, of_node_put()-under-devtree_lock, the
lan966x_pci_probe cleanup gap) are unrelated to this series and are
being tracked separately.

Verification
------------

Series was applied to v7.2-rc3 and boot-tested on an x86 platform
with two PCI-attached Xilinx FPGAs whose i2c-xiic controllers come
from driver-embedded DT overlays. Before this series, i2c bus
numbers auto-assigned regardless of what /aliases said and shifted
across boots depending on which FPGA won the probe race. After: 89
adapters, `/dev/i2c-N` numbering matches the overlay aliases exactly
and is stable across reboots, `sensors` reads live telemetry through
mux channels, and dmesg is free of WARN/BUG/Oops. Patch 4's unittest
passes.

[1] https://lore.kernel.org/lkml/1435675876-2159-1-git-send-email-geert+renesas@glider.be/
    Geert Uytterhoeven, "[PATCH/RFC 0/3] of/overlay: Update aliases
    when added or removed", 2015-06-30.
[2] Hervé Codina, "Using Device Tree Overlays to Support Complex PCI
    Devices in Linux", ELCE 2025.
    https://bootlin.com/pub/conferences/2025/elce/codina-pcie-dt-overlay.pdf

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
Abdurrahman Hussain (4):
      of: incrementally update /aliases lookup on reconfig notifications
      of/overlay: look up absolute target-paths absolutely
      of/overlay: rewrite /aliases path values to live-tree paths
      of: unittest: cover /aliases updates from overlay apply/revert

 drivers/of/base.c                           | 271 ++++++++++++++++++++++++----
 drivers/of/of_private.h                     |   7 +
 drivers/of/overlay.c                        |  61 +++++--
 drivers/of/unittest-data/Makefile           |   2 +
 drivers/of/unittest-data/overlay_alias.dtso |  36 ++++
 drivers/of/unittest.c                       |  74 ++++++++
 6 files changed, 393 insertions(+), 58 deletions(-)
---
base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
change-id: 20260719-nh-of-alias-overlay-6e5e0d56212b

Best regards,
--  
Abdurrahman Hussain <abdurrahman@nexthop.ai>

[PATCH RFC 1/4] of: incrementally update /aliases lookup on reconfig notifications

[Abdurrahman Hussain]

/aliases entries added by a device-tree overlay are stored in the live
tree but never enter the global aliases_lookup list that of_alias_scan()
builds at boot. As a result, of_alias_get_id() returns -ENODEV for
aliases declared inside overlays, and any driver that relies on
alias-based numbering (i2c-xiic, spi, tty, mmc, ...) silently loses its
pinned id and falls back to auto-assignment.

Fix by registering an internal OF reconfig notifier from
of_core_init() that mirrors /aliases changes into aliases_lookup:

  OF_RECONFIG_ADD_PROPERTY    -> of_alias_create()
  OF_RECONFIG_REMOVE_PROPERTY -> of_alias_destroy()
  OF_RECONFIG_UPDATE_PROPERTY -> destroy + create
  OF_RECONFIG_ATTACH_NODE     -> scan all properties (defensive)
  OF_RECONFIG_DETACH_NODE     -> forget all properties (defensive)

The reconfig notifier chain fires from both direct changesets and
overlay apply/revert, so the same code path covers runtime dt
modifications and overlay-declared aliases without any overlay-
specific hook in drivers/of/overlay.c. Grant Likely suggested this
shape on Geert Uytterhoeven's 2015 RFC [1]; Geert's original hook was
in dynamic.c directly.

Match the /aliases target node structurally (name == "aliases" and
parent == root) rather than by pointer against the of_aliases global.
A system with no boot-time /aliases has of_aliases == NULL, so an
overlay that creates /aliases from scratch would otherwise be missed
from the first ATTACH_NODE onward. ATTACH/DETACH also update
of_aliases lazily (with of_node_get/put) so subsequent consumers see
it.

Overlays build the node up empty and add properties via separate
ADD_PROPERTY events; other callers of of_attach_node() are allowed to
attach a fully-populated node in one shot. Handle both shapes by
scanning the node's properties on ATTACH_NODE and forgetting them on
DETACH_NODE. Overlay-driven flows still receive per-property events
and remain correct.

Factor the per-property loop body of of_alias_scan() into
of_alias_create() so the boot-time scan and the runtime notifier
share one code path. Owned (runtime) entries kstrdup the alias name
and hold an of_node_get() reference on the target so the alias_prop
survives the property that spawned it — required for the overlay
revert path where the source property is freed before we get a chance
to see it drop. A one-bit @owned flag on struct alias_prop distinguishes
kmalloc'd entries from memblock-backed ones so the destroy path
kfree()s the right ones.

The destroy path unlinks matching entries regardless of ownership
(freeing storage only for owned ones) so an overlay UPDATE against a
boot-time alias leaves at most one entry per stem+id. This addresses
the allocator-mismatch worry Grant flagged on the 2015 series [2] and
also the duplicate-mapping side effect that would otherwise leak
through.

Serialize aliases_lookup on a dedicated aliases_mutex: readers
(of_alias_get_id, of_alias_get_highest_id) and the reconfig notifier
both hold it around every access. Boot-time of_alias_scan() runs
single-threaded during init and stays lockless. This is preferable to
piggy-backing on of_mutex because the reconfig notifier is called
both under of_mutex (overlay apply path) and outside of it (direct
of_add_property() path from dynamic.c), so a nested acquisition would
deadlock on some callers.

Validate the property value before feeding it to of_find_node_by_path():
pp->value must be non-empty and null-terminated within pp->length. An
overlay that hasn't been through /aliases fixup can otherwise present
a fragment-internal string that isn't a valid live-tree path or a
malformed non-terminated value, and of_find_node_by_path() derefs it
as a C string — an OOB read on the malformed case.

Naming builds on Geert's original series:
  - "of: Extract of_alias_create()"                     [3]
  - "of: Add of_alias_destroy()"                        [4]
  - "of/dynamic: Update list of aliases on aliases changes" [5]

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]
Link: https://lore.kernel.org/lkml/1435675876-2159-2-git-send-email-geert+renesas@glider.be/ [3]
Link: https://lore.kernel.org/lkml/1435675876-2159-3-git-send-email-geert+renesas@glider.be/ [4]
Link: https://lore.kernel.org/lkml/1435675876-2159-4-git-send-email-geert+renesas@glider.be/ [5]
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/of/base.c       | 271 +++++++++++++++++++++++++++++++++++++++++-------
 drivers/of/of_private.h |   7 ++
 2 files changed, 238 insertions(+), 40 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 6e7a42dedad3..2695c5f8bb93 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1915,6 +1915,231 @@ static void of_alias_add(struct alias_prop *ap, struct device_node *np,
 		 ap->alias, ap->stem, ap->id, np);
 }
 
+/*
+ * Serializes aliases_lookup and of_aliases across boot-time scan,
+ * runtime notifier updates, and readers. of_alias_get_id() and
+ * of_alias_get_highest_id() acquire this before walking the list; the
+ * OF reconfig notifier below acquires it around each mutation.
+ * of_alias_scan() runs single-threaded from of_core_init() and skips
+ * the lock, but any code path that reads or writes aliases_lookup
+ * outside init must hold it.
+ */
+static DEFINE_MUTEX(aliases_mutex);
+
+/*
+ * Build an alias_prop for @pp using @dt_alloc as the storage allocator
+ * and add it to aliases_lookup. @owned is stored on the entry so the
+ * matching destroy path knows whether the alias_prop is a kmalloc'd
+ * struct that must be kfree()d (with a paired of_node_put on the
+ * target and kfree on the kstrdup'd alias name) or a memblock/
+ * dt_alloc'd struct that must be left alone.
+ *
+ * Callers other than of_alias_scan() must hold @aliases_mutex.
+ *
+ * Skips pseudo-properties (name, phandle, ...), malformed property
+ * values (empty or not null-terminated within pp->length), and alias
+ * names not ending in a numeric id.
+ */
+static void of_alias_create(const struct property *pp,
+			    void *(*dt_alloc)(u64 size, u64 align),
+			    bool owned)
+{
+	const char *start = pp->name;
+	const char *end;
+	struct device_node *np;
+	struct alias_prop *ap;
+	const char *dup;
+	int id, len;
+
+	if (is_pseudo_property(pp->name))
+		return;
+
+	/*
+	 * pp->value must be a non-empty, null-terminated string within
+	 * pp->length. of_find_node_by_path() derefs it as a C string, so
+	 * a missing terminator would cause an OOB read.
+	 */
+	if (!pp->value || pp->length < 2 ||
+	    strnlen(pp->value, pp->length) >= pp->length)
+		return;
+
+	np = of_find_node_by_path(pp->value);
+	if (!np)
+		return;
+
+	end = start + strlen(start);
+	while (end > start && isdigit(*(end - 1)))
+		end--;
+	len = end - start;
+	if (len == 0)
+		goto out_put;
+
+	if (kstrtoint(end, 10, &id) < 0)
+		goto out_put;
+
+	ap = dt_alloc(sizeof(*ap) + len + 1, __alignof__(*ap));
+	if (!ap)
+		goto out_put;
+	memset(ap, 0, sizeof(*ap) + len + 1);
+
+	/*
+	 * For runtime entries, kstrdup the alias name so the alias_prop
+	 * doesn't depend on pp->name remaining valid — an overlay revert
+	 * frees the source property. Boot-time entries point into the
+	 * FDT, which is never freed.
+	 */
+	if (owned) {
+		dup = kstrdup(pp->name, GFP_KERNEL);
+		if (!dup) {
+			kfree(ap);
+			goto out_put;
+		}
+	} else {
+		dup = start;
+	}
+	ap->alias = dup;
+	ap->owned = owned;
+	of_alias_add(ap, np, id, start, len);
+	return;
+
+out_put:
+	if (owned)
+		of_node_put(np);
+}
+
+/*
+ * Reverse of of_alias_create() for owned entries: unlink and free the
+ * matching alias_prop and drop the reference it holds on the target
+ * node. For boot-time entries (owned=false) it unlinks only — the
+ * struct and the alias name live in memblock and are never freed —
+ * which prevents an overlay-driven UPDATE_PROPERTY against a boot-time
+ * alias from leaving stale duplicates in aliases_lookup.
+ *
+ * Callers must hold @aliases_mutex.
+ */
+static void of_alias_destroy(const char *name)
+{
+	struct alias_prop *ap, *tmp;
+
+	list_for_each_entry_safe(ap, tmp, &aliases_lookup, link) {
+		if (strcmp(ap->alias, name) != 0)
+			continue;
+		list_del(&ap->link);
+		if (ap->owned) {
+			of_node_put(ap->np);
+			kfree(ap->alias);
+			kfree(ap);
+		}
+		return;
+	}
+}
+
+static void *alias_alloc(u64 size, u64 align)
+{
+	return kzalloc(size, GFP_KERNEL);
+}
+
+/* Scan every property of @aliases and mirror it into aliases_lookup. */
+static void of_alias_node_scan(struct device_node *aliases)
+{
+	struct property *pp;
+
+	for_each_property_of_node(aliases, pp)
+		of_alias_create(pp, alias_alloc, true);
+}
+
+/* Reverse: destroy every alias_prop backed by a property of @aliases. */
+static void of_alias_node_forget(struct device_node *aliases)
+{
+	struct property *pp;
+
+	for_each_property_of_node(aliases, pp)
+		of_alias_destroy(pp->name);
+}
+
+/*
+ * OF reconfig notifier that mirrors /aliases property changes into
+ * aliases_lookup. Fires on both direct changesets and overlay
+ * apply/revert, so of_alias_get_id() returns the right id for aliases
+ * declared inside an overlay.
+ */
+static int of_aliases_reconfig_notifier(struct notifier_block *nb,
+					unsigned long action, void *arg)
+{
+	struct of_reconfig_data *rd = arg;
+
+	/*
+	 * Match /aliases structurally (name + root-parent) rather than by
+	 * pointer against the of_aliases global — a system with no
+	 * boot-time /aliases (of_aliases == NULL) can still acquire one
+	 * from an overlay, and we must track its properties from the
+	 * first ATTACH_NODE onward.
+	 */
+	if (!rd->dn || !rd->dn->parent ||
+	    !of_node_is_root(rd->dn->parent) ||
+	    !of_node_name_eq(rd->dn, "aliases"))
+		return NOTIFY_DONE;
+
+	mutex_lock(&aliases_mutex);
+	switch (action) {
+	case OF_RECONFIG_ATTACH_NODE:
+		/*
+		 * Overlays build the node up empty and add properties via
+		 * separate ADD_PROPERTY events, but the reconfig API also
+		 * permits attaching a fully-populated node in one shot —
+		 * scan defensively so pre-populated aliases aren't lost.
+		 */
+		of_alias_node_scan(rd->dn);
+		if (!of_aliases)
+			of_aliases = of_node_get(rd->dn);
+		break;
+	case OF_RECONFIG_DETACH_NODE:
+		/*
+		 * Symmetric with ATTACH_NODE: some callers detach without
+		 * emitting per-property REMOVE events first, so drop every
+		 * alias_prop backed by this node's properties before it
+		 * disappears.
+		 */
+		of_alias_node_forget(rd->dn);
+		if (of_aliases == rd->dn) {
+			of_node_put(of_aliases);
+			of_aliases = NULL;
+		}
+		break;
+	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;
+	default:
+		break;
+	}
+	mutex_unlock(&aliases_mutex);
+	return NOTIFY_OK;
+}
+
+static struct notifier_block of_aliases_nb = {
+	.notifier_call = of_aliases_reconfig_notifier,
+};
+
+static int __init of_aliases_reconfig_init(void)
+{
+	return of_reconfig_notifier_register(&of_aliases_nb);
+}
+
+/*
+ * of_alias_scan() runs from of_core_init() (core_initcall), so hook the
+ * reconfig notifier one initcall level later to guarantee the initial
+ * static scan is complete before any dynamic tracking begins.
+ */
+core_initcall_sync(of_aliases_reconfig_init);
+
 /**
  * of_alias_scan - Scan all properties of the 'aliases' node
  * @dt_alloc:	An allocator that provides a virtual address to memory
@@ -1950,42 +2175,8 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
 	if (!of_aliases)
 		return;
 
-	for_each_property_of_node(of_aliases, pp) {
-		const char *start = pp->name;
-		const char *end = start + strlen(start);
-		struct device_node *np;
-		struct alias_prop *ap;
-		int id, len;
-
-		/* Skip those we do not want to proceed */
-		if (is_pseudo_property(pp->name))
-			continue;
-
-		np = of_find_node_by_path(pp->value);
-		if (!np)
-			continue;
-
-		/* walk the alias backwards to extract the id and work out
-		 * the 'stem' string */
-		while (isdigit(*(end-1)) && end > start)
-			end--;
-		len = end - start;
-
-		if (kstrtoint(end, 10, &id) < 0) {
-			of_node_put(np);
-			continue;
-		}
-
-		/* Allocate an alias_prop with enough space for the stem */
-		ap = dt_alloc(sizeof(*ap) + len + 1, __alignof__(*ap));
-		if (!ap) {
-			of_node_put(np);
-			continue;
-		}
-		memset(ap, 0, sizeof(*ap) + len + 1);
-		ap->alias = start;
-		of_alias_add(ap, np, id, start, len);
-	}
+	for_each_property_of_node(of_aliases, pp)
+		of_alias_create(pp, dt_alloc, false);
 }
 
 /**
@@ -2003,7 +2194,7 @@ int of_alias_get_id(const struct device_node *np, const char *stem)
 	struct alias_prop *app;
 	int id = -ENODEV;
 
-	mutex_lock(&of_mutex);
+	mutex_lock(&aliases_mutex);
 	list_for_each_entry(app, &aliases_lookup, link) {
 		if (strcmp(app->stem, stem) != 0)
 			continue;
@@ -2013,7 +2204,7 @@ int of_alias_get_id(const struct device_node *np, const char *stem)
 			break;
 		}
 	}
-	mutex_unlock(&of_mutex);
+	mutex_unlock(&aliases_mutex);
 
 	return id;
 }
@@ -2031,7 +2222,7 @@ int of_alias_get_highest_id(const char *stem)
 	struct alias_prop *app;
 	int id = -ENODEV;
 
-	mutex_lock(&of_mutex);
+	mutex_lock(&aliases_mutex);
 	list_for_each_entry(app, &aliases_lookup, link) {
 		if (strcmp(app->stem, stem) != 0)
 			continue;
@@ -2039,7 +2230,7 @@ int of_alias_get_highest_id(const char *stem)
 		if (app->id > id)
 			id = app->id;
 	}
-	mutex_unlock(&of_mutex);
+	mutex_unlock(&aliases_mutex);
 
 	return id;
 }
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 0ae16da066e2..9d16765ae2c3 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -17,6 +17,12 @@
  * @alias:	Alias property name
  * @np:		Pointer to device_node that the alias stands for
  * @id:		Index value from end of alias name
+ * @owned:	True if @alias was kstrdup'd and @np was of_node_get'd on
+ *		insertion (overlay-time entries). False for entries built
+ *		by of_alias_scan() at boot, where @alias points into the
+ *		FDT and @np is an unreferenced pointer. The removal path
+ *		uses this flag to decide whether it must kfree(@alias),
+ *		of_node_put(@np), and kfree(the struct itself).
  * @stem:	Alias string without the index
  *
  * The structure represents one alias property of 'aliases' node as
@@ -27,6 +33,7 @@ struct alias_prop {
 	const char *alias;
 	struct device_node *np;
 	int id;
+	bool owned;
 	char stem[];
 };
 

-- 
2.54.0

[PATCH RFC 2/4] of/overlay: look up absolute target-paths absolutely

[Abdurrahman Hussain]

When of_overlay_fdt_apply() is called with a non-NULL target base,
find_target() currently concatenates the base's full path with every
fragment's target-path via "%pOF%s" — so target-path="" resolves to
the base itself (the intended common case), but target-path="/foo"
resolves to "<base>/foo" (never the DT root) and target-path="/" to
"<base>/" (never a valid node at all).

That makes it impossible for a two-fragment overlay to modify one
subtree under the base and one node at the DT root — a shape that
arises naturally when a PCI-attached device wants to declare its
peripherals under dev_of_node(&pdev->dev) AND add /aliases entries
so alias-aware drivers (i2c-xiic, spi, tty, ...) can pin bus numbers.

Treat target-path as absolute whenever it is non-empty. An empty
target-path continues to mean "the target base itself", preserving
the existing shape used by drivers/misc/lan966x_pci.c and its dtso
(the only in-tree of_overlay_fdt_apply() caller today that passes a
non-NULL base).

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/of/overlay.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 08d5351746be..654a70d5cb07 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -693,7 +693,6 @@ static struct device_node *find_target(const struct device_node *info_node,
 				       const struct device_node *target_base)
 {
 	struct device_node *node;
-	char *target_path;
 	const char *path;
 	u32 val;
 	int ret;
@@ -709,23 +708,22 @@ static struct device_node *find_target(const struct device_node *info_node,
 
 	ret = of_property_read_string(info_node, "target-path", &path);
 	if (!ret) {
-		if (target_base) {
-			target_path = kasprintf(GFP_KERNEL, "%pOF%s", target_base, path);
-			if (!target_path)
-				return NULL;
-			node = of_find_node_by_path(target_path);
-			if (!node) {
-				pr_err("find target, node: %pOF, path '%s' not found\n",
-				       info_node, target_path);
-			}
-			kfree(target_path);
-		} else {
-			node =  of_find_node_by_path(path);
-			if (!node) {
-				pr_err("find target, node: %pOF, path '%s' not found\n",
-				       info_node, path);
-			}
-		}
+		/*
+		 * With a non-NULL @target_base, an empty target-path means
+		 * "the target base itself" — this is the common
+		 * of_overlay_fdt_apply(..., base) form used by e.g. the
+		 * LAN966x PCI overlay. Any other target-path is looked up
+		 * absolutely, so overlays that also need to reach the DT
+		 * root (e.g. to add /aliases entries alongside a base-
+		 * relative fragment) can do so with target-path="/aliases".
+		 */
+		if (target_base && path[0] == '\0')
+			return of_node_get((struct device_node *)target_base);
+
+		node = of_find_node_by_path(path);
+		if (!node)
+			pr_err("find target, node: %pOF, path '%s' not found\n",
+			       info_node, path);
 		return node;
 	}
 

-- 
2.54.0

[PATCH RFC 3/4] of/overlay: rewrite /aliases path values to live-tree paths

[Abdurrahman Hussain]

/aliases entries added by an overlay reference labeled nodes inside
the overlay via '&label' in the .dtso. dtc renders those references
as string paths at compile time, but the paths encode the overlay's
internal fragment layout (e.g. "/fragment@1/__overlay__/fpga@0/i2c@40000")
rather than the location where the node will live after apply.

Currently only /__symbols__ has its property values rewritten from
overlay-internal paths to live-tree paths by dup_and_fixup_symbol_prop().
/aliases values fall through the plain __of_prop_dup() path and are
copied byte-for-byte, so of_find_node_by_path() on such a value returns
NULL, of_alias_get_id() reports -ENODEV — and the reconfig notifier
added earlier in this series sees uninterpretable paths and can't
populate aliases_lookup for overlay-declared aliases.

The values in /aliases follow the same textual convention as
/__symbols__, so we can reuse the existing rewriter. Detect the
/aliases target node and route its properties through
dup_and_fixup_symbol_prop() only when the value actually looks like a
fragment-internal path (non-empty, null-terminated within pp->length,
"/fragment@" prefix); other alias values — legacy string aliases like
"ttyS0" that some out-of-tree code writes verbatim — pass through the
plain __of_prop_dup() path unchanged.

Discriminating up-front rather than falling back on a NULL return
matters because dup_and_fixup_symbol_prop() also returns NULL on
allocation failure and on malformed values. A blanket fallback would
mask -ENOMEM as "not our shape" and, worse, would copy a
non-null-terminated value byte-for-byte into the live tree — where
the alias reconfig notifier's downstream of_find_node_by_path() would
treat it as a C string and read past the end. Structural validation
here means a NULL from dup_and_fixup_symbol_prop() below can only be
-ENOMEM and is propagated as such.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/of/overlay.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 654a70d5cb07..69358811a7f9 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -350,6 +350,33 @@ static int add_changeset_property(struct overlay_changeset *ovcs,
 		if (prop)
 			return -EINVAL;
 		new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop);
+	} else if (target->np->parent &&
+		   of_node_is_root(target->np->parent) &&
+		   of_node_name_eq(target->np, "aliases") &&
+		   overlay_prop->length >= sizeof("/fragment@") &&
+		   overlay_prop->value &&
+		   strnlen(overlay_prop->value, overlay_prop->length) <
+			overlay_prop->length &&
+		   !strncmp(overlay_prop->value, "/fragment@",
+			    strlen("/fragment@"))) {
+		/*
+		 * /aliases property values that reference a labeled node
+		 * inside this overlay are rendered by dtc as string paths
+		 * "/fragment@N/__overlay__/..." — the overlay's internal
+		 * layout rather than where the node lives after apply.
+		 * Reuse dup_and_fixup_symbol_prop() (which already handles
+		 * this rewrite for /__symbols__) so of_alias_get_id() can
+		 * resolve the value in the live tree.
+		 *
+		 * The property-value shape (non-empty, null-terminated
+		 * within pp->length, prefix "/fragment@") is validated
+		 * above so a NULL return from dup_and_fixup_symbol_prop()
+		 * here means -ENOMEM, not "not our shape"; let the shared
+		 * -ENOMEM check below handle it rather than falling back
+		 * to a raw dup that would inject an unresolvable path
+		 * into the live tree.
+		 */
+		new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop);
 	} else {
 		new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL);
 	}

-- 
2.54.0

[PATCH RFC 4/4] of: unittest: cover /aliases updates from overlay apply/revert

[Abdurrahman Hussain]

Add overlay_alias.dtso plus of_unittest_overlay_alias() to cover the
"aliases inside an overlay" flow end-to-end.

The overlay has two fragments:

  fragment@0: target-path="" grafts a labeled node under target_base.
  fragment@1: target-path="/aliases" adds `testcase-alias99 =
              &<label>` at DT-root /aliases.

The runner applies the overlay with a non-NULL target_base via
of_overlay_fdt_apply() directly (rather than the overlay_data_apply()
wrapper, which always passes NULL), then asserts of_alias_get_id() on
the grafted node returns 99 across apply and -ENODEV across revert.

Exercises all three functional patches earlier in this series
together:

  * the reconfig notifier is what mutates aliases_lookup on
    apply/revert;
  * find_target()'s absolute-target-path handling is what lets
    fragment@1's target-path="/aliases" reach the DT root when
    target_base is non-NULL;
  * dup_and_fixup_symbol_prop()'s /aliases branch is what rewrites
    the fragment-internal alias value ("&<label>" -> fragment path)
    into the live-tree path that of_find_node_by_path() can resolve.

Any one of the three missing turns the middle assertion (get_id -> 99)
into -ENODEV.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/of/unittest-data/Makefile           |  2 +
 drivers/of/unittest-data/overlay_alias.dtso | 36 ++++++++++++++
 drivers/of/unittest.c                       | 74 +++++++++++++++++++++++++++++
 3 files changed, 112 insertions(+)

diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
index 01a966e39f23..0a8bd9a74283 100644
--- a/drivers/of/unittest-data/Makefile
+++ b/drivers/of/unittest-data/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_OF_OVERLAY) += overlay.dtbo.o \
 			    overlay_18.dtbo.o \
 			    overlay_19.dtbo.o \
 			    overlay_20.dtbo.o \
+			    overlay_alias.dtbo.o \
 			    overlay_bad_add_dup_node.dtbo.o \
 			    overlay_bad_add_dup_prop.dtbo.o \
 			    overlay_bad_phandle.dtbo.o \
@@ -87,6 +88,7 @@ apply_static_overlay_1 := overlay_0.dtbo \
 			  overlay_18.dtbo \
 			  overlay_19.dtbo \
 			  overlay_20.dtbo \
+			  overlay_alias.dtbo \
 			  overlay_gpio_01.dtbo \
 			  overlay_gpio_02a.dtbo \
 			  overlay_gpio_02b.dtbo \
diff --git a/drivers/of/unittest-data/overlay_alias.dtso b/drivers/of/unittest-data/overlay_alias.dtso
new file mode 100644
index 000000000000..7763fffa8699
--- /dev/null
+++ b/drivers/of/unittest-data/overlay_alias.dtso
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+/*
+ * overlay_alias - exercise the full "aliases inside an overlay" path.
+ *
+ * fragment@0 grafts a labeled node under target_base (empty target-path
+ * means "the target base itself"). fragment@1 adds a numbered alias at
+ * the DT-root /aliases and points it at that label. target-path="/aliases"
+ * is an absolute path — reaching it while target_base is non-NULL
+ * exercises the absolute-target-paths handling in find_target(). The
+ * alias value is emitted by dtc as the fragment-internal path
+ * "/fragment@0/__overlay__/alias-target"; the overlay code's
+ * /aliases path rewrite is what makes it resolve in the live tree.
+ * The reconfig notifier is what mirrors the change into aliases_lookup.
+ */
+
+/ {
+	fragment@0 {
+		target-path = "";
+		__overlay__ {
+			testcase-target: alias-target {
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+		};
+	};
+
+	fragment@1 {
+		target-path = "/aliases";
+		__overlay__ {
+			testcase-alias99 = &testcase-target;
+		};
+	};
+};
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index e255f54f4d76..35d75f2b3ef2 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -3475,6 +3475,76 @@ static struct notifier_block of_nb = {
 	.notifier_call = of_notify,
 };
 
+static void __init of_unittest_overlay_alias(void)
+{
+	const char *base_path =
+		"/testcase-data/overlay-node/test-bus/test-unittest100";
+	const char *target_path =
+		"/testcase-data/overlay-node/test-bus/test-unittest100/alias-target";
+	struct device_node *base, *target;
+	struct overlay_info *info;
+	int ovcs_id = 0;
+	int id, ret;
+	u32 size;
+
+	/*
+	 * Apply the overlay with a non-NULL target_base so that
+	 * fragment@1's target-path="/aliases" is looked up absolutely
+	 * (exercising find_target()'s absolute-path handling) rather
+	 * than under the base. The alias value inside the overlay is a
+	 * fragment-internal path that only resolves after the overlay
+	 * code rewrites /aliases values, so this run covers the whole
+	 * chain end-to-end.
+	 */
+	base = of_find_node_by_path(base_path);
+	if (!base) {
+		unittest(0, "could not find alias target base %s\n", base_path);
+		return;
+	}
+
+	for (info = overlays; info && info->name; info++)
+		if (!strcmp("overlay_alias", info->name))
+			break;
+	if (!info || !info->name) {
+		unittest(0, "no overlay data for overlay_alias\n");
+		goto out_put_base;
+	}
+
+	size = info->dtbo_end - info->dtbo_begin;
+	ret = of_overlay_fdt_apply(info->dtbo_begin, size, &ovcs_id, base);
+	if (ret < 0) {
+		unittest(0, "overlay_alias apply failed, ret = %d\n", ret);
+		goto out_put_base;
+	}
+
+	target = of_find_node_by_path(target_path);
+	if (!target) {
+		unittest(0, "could not find grafted target %s\n", target_path);
+		goto out_remove;
+	}
+
+	id = of_alias_get_id(target, "testcase-alias");
+	unittest(id == 99,
+		 "of_alias_get_id() = %d after overlay apply, expected 99\n",
+		 id);
+	of_node_put(target);
+
+out_remove:
+	ret = of_overlay_remove(&ovcs_id);
+	if (ret) {
+		unittest(0, "overlay_alias remove failed, ret = %d\n", ret);
+		goto out_put_base;
+	}
+
+	id = of_alias_get_id(base, "testcase-alias");
+	unittest(id == -ENODEV,
+		 "of_alias_get_id() = %d after overlay remove, expected -ENODEV\n",
+		 id);
+
+out_put_base:
+	of_node_put(base);
+}
+
 static void __init of_unittest_overlay_notify(void)
 {
 	int ovcs_id;
@@ -3649,6 +3719,8 @@ static void __init of_unittest_overlay(void)
 
 	of_unittest_overlay_gpio();
 
+	of_unittest_overlay_alias();
+
 	of_unittest_remove_tracked_overlays();
 
 	of_unittest_overlay_notify();
@@ -3848,6 +3920,7 @@ OVERLAY_INFO_EXTERN(overlay_17);
 OVERLAY_INFO_EXTERN(overlay_18);
 OVERLAY_INFO_EXTERN(overlay_19);
 OVERLAY_INFO_EXTERN(overlay_20);
+OVERLAY_INFO_EXTERN(overlay_alias);
 OVERLAY_INFO_EXTERN(overlay_gpio_01);
 OVERLAY_INFO_EXTERN(overlay_gpio_02a);
 OVERLAY_INFO_EXTERN(overlay_gpio_02b);
@@ -3885,6 +3958,7 @@ static struct overlay_info overlays[] = {
 	OVERLAY_INFO(overlay_18, 0, 0),
 	OVERLAY_INFO(overlay_19, 0, 0),
 	OVERLAY_INFO(overlay_20, 0, 0),
+	OVERLAY_INFO(overlay_alias, 0, 0),
 	OVERLAY_INFO(overlay_gpio_01, 0, 0),
 	OVERLAY_INFO(overlay_gpio_02a, 0, 0),
 	OVERLAY_INFO(overlay_gpio_02b, 0, 0),

-- 
2.54.0

Re: [PATCH RFC 4/4] of: unittest: cover /aliases updates from overlay apply/revert

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The test assertion for verifying alias removal trivially passes because it checks the wrong node, rendering the removal test ineffective.
- [Medium] The error path for `of_overlay_fdt_apply()` skips cleanup, leading to a memory and device tree state leak.
--

--- Patch [4]: [PATCH RFC 4/4] of: unittest: cover /aliases updates from overlay apply/revert ---
commit d95c5208da27c0c2461530192533ee49d2b62c1c
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

of: unittest: cover /aliases updates from overlay apply/revert

This commit adds overlay_alias.dtso and of_unittest_overlay_alias() to
cover the end-to-end flow of aliases inside an overlay. It verifies alias
updates when applying and reverting overlays with a non-NULL target base.

> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index e255f54f4d760..35d75f2b3ef25 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c

[ ... ]

> +	size = info->dtbo_end - info->dtbo_begin;
> +	ret = of_overlay_fdt_apply(info->dtbo_begin, size, &ovcs_id, base);
> +	if (ret < 0) {
> +		unittest(0, "overlay_alias apply failed, ret = %d\n", ret);
> +		goto out_put_base;

[Severity: Medium]
Does this error path skip cleaning up the changeset?

Looking at of_overlay_fdt_apply(), if it fails during the apply phase, it
may still populate ovcs_id. Does this code need to call of_overlay_remove()
to clean up the partial changeset before jumping to out_put_base, to avoid
leaking memory and device tree state?

> +	}
> +
> +	target = of_find_node_by_path(target_path);
> +	if (!target) {
> +		unittest(0, "could not find grafted target %s\n", target_path);
> +		goto out_remove;
> +	}
> +
> +	id = of_alias_get_id(target, "testcase-alias");
> +	unittest(id == 99,
> +		 "of_alias_get_id() = %d after overlay apply, expected 99\n",
> +		 id);
> +	of_node_put(target);
> +
> +out_remove:
> +	ret = of_overlay_remove(&ovcs_id);
> +	if (ret) {
> +		unittest(0, "overlay_alias remove failed, ret = %d\n", ret);
> +		goto out_put_base;
> +	}
> +
> +	id = of_alias_get_id(base, "testcase-alias");

[Severity: Medium]
Does this code test the correct node during the removal assertion?

The assertion checks 'base' for the alias, but the earlier success path
checked 'target'. Doesn't of_alias_get_id(base, "testcase-alias") always
return -ENODEV here, meaning the test will pass even if the overlay removal
failed to actually clean up the alias?

> +	unittest(id == -ENODEV,
> +		 "of_alias_get_id() = %d after overlay remove, expected -ENODEV\n",
> +		 id);
> +
> +out_put_base:
> +	of_node_put(base);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-nh-of-alias-overlay-v1-0-27da6848dd84@nexthop.ai?part=4

Re: [PATCH RFC 1/4] of: incrementally update /aliases lookup on reconfig notifications

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Data race and possible list corruption because `of_device_uevent()` iterates over `aliases_lookup` while holding `of_mutex` instead of the newly introduced `aliases_mutex`.
- [High] Lockless iteration of `aliases->properties` in `of_alias_node_scan()` and `of_alias_node_forget()` can race with concurrent property updates.
- [Medium] Missing `of_node_put()` leaks target device node references on error paths in `of_alias_create()` and during overlay updates of boot-time aliases in `of_alias_destroy()`.
--

--- Patch [1]: [PATCH RFC 1/4] of: incrementally update /aliases lookup on reconfig notifications ---
commit 0e641f78493b9291f518b7ebe305e2c1cbac0e2b
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

of: incrementally update /aliases lookup on reconfig notifications
    
/aliases entries added by a device-tree overlay are stored in the live
tree but never enter the global aliases_lookup list that of_alias_scan()
builds at boot. As a result, of_alias_get_id() returns -ENODEV for
aliases declared inside overlays, and any driver that relies on
alias-based numbering (i2c-xiic, spi, tty, mmc, ...) silently loses its
pinned id and falls back to auto-assignment.

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]
Link: https://lore.kernel.org/lkml/1435675876-2159-2-git-send-email-geert+renesas@glider.be/ [3]
Link: https://lore.kernel.org/lkml/1435675876-2159-3-git-send-email-geert+renesas@glider.be/ [4]
Link: https://lore.kernel.org/lkml/1435675876-2159-4-git-send-email-geert+renesas@glider.be/ [5]

> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 6e7a42dedad3f..2695c5f8bb93a 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1915,6 +1915,231 @@ static void of_alias_add(struct alias_prop *ap, struct device_node *np,
>  		 ap->alias, ap->stem, ap->id, np);
>  }
>  
> +/*
> + * Serializes aliases_lookup and of_aliases across boot-time scan,
> + * runtime notifier updates, and readers. of_alias_get_id() and
> + * of_alias_get_highest_id() acquire this before walking the list; the
> + * OF reconfig notifier below acquires it around each mutation.
> + * of_alias_scan() runs single-threaded from of_core_init() and skips
> + * the lock, but any code path that reads or writes aliases_lookup
> + * outside init must hold it.
> + */
> +static DEFINE_MUTEX(aliases_mutex);

[Severity: High]
Does this new mutex also need to be acquired in of_device_uevent()?

Looking at of_device_uevent() in drivers/of/device.c, it still iterates over 
aliases_lookup using of_mutex:

drivers/of/device.c:of_device_uevent() {
    ...
	mutex_lock(&of_mutex);
	list_for_each_entry(app, &aliases_lookup, link) {
		if (dev->of_node == app->np) {
    ...
	}
	mutex_unlock(&of_mutex);
}

Since of_aliases_reconfig_notifier() modifies the list while holding only 
aliases_mutex, could this result in concurrent modification and list corruption 
if a uevent is triggered during an overlay update?

[ ... ]

> +	if (kstrtoint(end, 10, &id) < 0)
> +		goto out_put;
> +
> +	ap = dt_alloc(sizeof(*ap) + len + 1, __alignof__(*ap));
> +	if (!ap)
> +		goto out_put;
> +	memset(ap, 0, sizeof(*ap) + len + 1);
> +
> +	/*
> +	 * For runtime entries, kstrdup the alias name so the alias_prop
> +	 * doesn't depend on pp->name remaining valid — an overlay revert
> +	 * frees the source property. Boot-time entries point into the
> +	 * FDT, which is never freed.
> +	 */
> +	if (owned) {
> +		dup = kstrdup(pp->name, GFP_KERNEL);
> +		if (!dup) {
> +			kfree(ap);
> +			goto out_put;
> +		}
> +	} else {
> +		dup = start;
> +	}
> +	ap->alias = dup;
> +	ap->owned = owned;
> +	of_alias_add(ap, np, id, start, len);
> +	return;
> +
> +out_put:
> +	if (owned)
> +		of_node_put(np);
> +}

[Severity: Medium]
Does this leak the device node reference for boot-time aliases on error?

When of_find_node_by_path() is called earlier in this function, it acquires a 
reference to the returned node regardless of whether the alias is owned or not.
If an error occurs (like kstrtoint() or dt_alloc() failing) for a boot-time 
alias where owned is false, the out_put label is reached but of_node_put(np) 
is skipped, leaking the reference.

> +/*
> + * Reverse of of_alias_create() for owned entries: unlink and free the
> + * matching alias_prop and drop the reference it holds on the target
> + * node. For boot-time entries (owned=false) it unlinks only — the
> + * struct and the alias name live in memblock and are never freed —
> + * which prevents an overlay-driven UPDATE_PROPERTY against a boot-time
> + * alias from leaving stale duplicates in aliases_lookup.
> + *
> + * Callers must hold @aliases_mutex.
> + */
> +static void of_alias_destroy(const char *name)
> +{
> +	struct alias_prop *ap, *tmp;
> +
> +	list_for_each_entry_safe(ap, tmp, &aliases_lookup, link) {
> +		if (strcmp(ap->alias, name) != 0)
> +			continue;
> +		list_del(&ap->link);
> +		if (ap->owned) {
> +			of_node_put(ap->np);
> +			kfree(ap->alias);
> +			kfree(ap);
> +		}
> +		return;
> +	}
> +}

[Severity: Medium]
Does this also leak the device node reference if a boot-time alias is updated
or removed?

If an overlay updates a boot-time alias (where ap->owned is false), the alias 
is unlinked from the list, but of_node_put(ap->np) is skipped. Since the 
reference was acquired at boot time by of_find_node_by_path(), doesn't it 
need to be released when the alias is destroyed?

> +static void *alias_alloc(u64 size, u64 align)
> +{
> +	return kzalloc(size, GFP_KERNEL);
> +}
> +
> +/* Scan every property of @aliases and mirror it into aliases_lookup. */
> +static void of_alias_node_scan(struct device_node *aliases)
> +{
> +	struct property *pp;
> +
> +	for_each_property_of_node(aliases, pp)
> +		of_alias_create(pp, alias_alloc, true);
> +}
> +
> +/* Reverse: destroy every alias_prop backed by a property of @aliases. */
> +static void of_alias_node_forget(struct device_node *aliases)
> +{
> +	struct property *pp;
> +
> +	for_each_property_of_node(aliases, pp)
> +		of_alias_destroy(pp->name);
> +}

[Severity: High]
Is it safe to iterate aliases->properties without holding devtree_lock?

These functions use for_each_property_of_node() to traverse the properties list 
locklessly. Since of_aliases_reconfig_notifier() is called for ATTACH_NODE 
and DETACH_NODE events, could a concurrent thread adding or removing properties 
on the aliases node mutate the linked list under devtree_lock, causing a 
race condition or traversing stale pointers here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-nh-of-alias-overlay-v1-0-27da6848dd84@nexthop.ai?part=1

Re: [PATCH RFC 3/4] of/overlay: rewrite /aliases path values to live-tree paths

[sashiko-bot]

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

New issues:
- [Medium] A structural validation failure in `dup_and_fixup_symbol_prop()` is incorrectly masked as a memory allocation failure.

Pre-existing issues:
- [High] The `__of_prop_dup` fallback permits non-null-terminated alias strings to enter the live tree, causing a Slab Out-Of-Bounds Read.
- [High] Double Free / Use-After-Free of `new_prop` on memory allocation failure.
- [High] Memory leak of successfully processed device nodes on early exit in `init_overlay_changeset()`.
--

--- Patch [3]: [PATCH RFC 3/4] of/overlay: rewrite /aliases path values to live-tree paths ---
commit ac88ca3a222d2b48e52178c676efb92de0cef66b
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>
Subject: of/overlay: rewrite /aliases path values to live-tree paths

The commit rewrites /aliases entries added by an overlay to use live-tree
paths instead of overlay-internal fragment paths. It checks if the alias
value starts with /fragment@ and uses dup_and_fixup_symbol_prop() to fix
it up. Other alias values fall through to __of_prop_dup().

> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 654a70d5cb075..69358811a7f94 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -350,6 +350,33 @@ static int add_changeset_property(struct overlay_changeset *ovcs,
>  		if (prop)
>  			return -EINVAL;
>  		new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop);
> +	} else if (target->np->parent &&
> +		   of_node_is_root(target->np->parent) &&
> +		   of_node_name_eq(target->np, "aliases") &&
> +		   overlay_prop->length >= sizeof("/fragment@") &&
> +		   overlay_prop->value &&
> +		   strnlen(overlay_prop->value, overlay_prop->length) <
> +			overlay_prop->length &&
> +		   !strncmp(overlay_prop->value, "/fragment@",
> +			    strlen("/fragment@"))) {

[Severity: High]
This isn't a bug introduced by this patch, but does this logic still allow
non-null-terminated strings to enter the live tree?

If an overlay provides an /aliases property with a non-null-terminated
string that does not start with /fragment@ (e.g., ttyS0 missing a null byte),
the else if condition evaluates to false.

Execution falls through to the else block:

drivers/of/overlay.c:add_changeset_property() {
    ...
	} else {
		new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL);
	}
    ...
}

Here __of_prop_dup() blindly copies the non-null-terminated bytes directly
into the live device tree. When a caller such as of_find_node_opts_by_path()
attempts to resolve the alias, it passes pp->value to of_find_node_by_path(),
which uses standard C string functions (strncmp, strchr) on the buffer,
reading past the end.

> +		/*
> +		 * /aliases property values that reference a labeled node
> +		 * inside this overlay are rendered by dtc as string paths
> +		 * "/fragment@N/__overlay__/..." — the overlay's internal
> +		 * layout rather than where the node lives after apply.
> +		 * Reuse dup_and_fixup_symbol_prop() (which already handles
> +		 * this rewrite for /__symbols__) so of_alias_get_id() can
> +		 * resolve the value in the live tree.
> +		 *
> +		 * The property-value shape (non-empty, null-terminated
> +		 * within pp->length, prefix "/fragment@") is validated
> +		 * above so a NULL return from dup_and_fixup_symbol_prop()
> +		 * here means -ENOMEM, not "not our shape"; let the shared
> +		 * -ENOMEM check below handle it rather than falling back
> +		 * to a raw dup that would inject an unresolvable path
> +		 * into the live tree.
> +		 */
> +		new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop);

[Severity: Medium]
Does this incorrectly mask a structural validation failure as a memory
allocation failure?

When an overlay provides a structurally valid /aliases property that starts
with /fragment@ and is null-terminated, but references a non-existent
overlay fragment, dup_and_fixup_symbol_prop() will legitimately return NULL:

drivers/of/overlay.c:dup_and_fixup_symbol_prop() {
    ...
	if (k >= ovcs->count)
		return NULL;
    ...
}

The shared error handler below then incorrectly converts this structural
failure into a memory exhaustion error (-ENOMEM).

>  	} else {
>  		new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL);
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but can this cause a double free
of new_prop on memory allocation failure?

If !target->in_livetree, new_prop is linked into target->np->deadprops:

drivers/of/overlay.c:add_changeset_property() {
    ...
	if (!prop) {
		if (!target->in_livetree) {
			new_prop->next = target->np->deadprops;
			target->np->deadprops = new_prop;
		}
		ret = of_changeset_add_property(&ovcs->cset, target->np,
						new_prop);
	}
    ...
}

If of_changeset_add_property() returns an error, the function explicitly
calls __of_prop_free(new_prop):

drivers/of/overlay.c:add_changeset_property() {
    ...
	if (ret)
		__of_prop_free(new_prop);
	return ret;
}

Since new_prop was not unlinked from the deadprops list, when the overlay
application aborts and of_node_release() iterates over deadprops, will it
call __of_prop_free() again on the already-freed pointer?

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but is there a memory leak of
successfully processed device nodes on early exit in
init_overlay_changeset()?

When applying a multi-fragment device tree overlay, if an intermediate
fragment fails find_target() (e.g., due to an invalid target-path), the
code jumps to err_out:

drivers/of/overlay.c:init_overlay_changeset() {
    ...
		fragment->target = find_target(node, target_base);
		if (!fragment->target) {
			of_node_put(fragment->overlay);
			ret = -EINVAL;
			of_node_put(node);
			goto err_out;
		}
    ...
}

As the function iterates over the fragments, it increments the reference
counts for both the overlay and target device nodes, storing them in the
fragments array. When find_target() fails for a subsequent fragment, the
function exits with an error without updating ovcs->count (which remains 0).

The cleanup function free_overlay_changeset() relies on ovcs->count to loop
through and release the node references. Because the count is 0, the cleanup
loop is skipped entirely, causing a permanent leak of the successfully
processed node references from the prior fragments.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-nh-of-alias-overlay-v1-0-27da6848dd84@nexthop.ai?part=3