Devicetree
 help / color / mirror / Atom feed
* [PATCH v3 0/4] of: teach overlay code to keep /aliases in sync
@ 2026-07-21 21:36 Abdurrahman Hussain
  2026-07-21 21:36 ` [PATCH v3 1/4] of: incrementally update /aliases lookup on reconfig notifications Abdurrahman Hussain
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Abdurrahman Hussain @ 2026-07-21 21:36 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan
  Cc: devicetree, linux-kernel, 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 aliases_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 v3
-------------

Addresses the automated review of v2.

Patch 1:
  - of_device_uevent() in drivers/of/device.c now walks aliases_lookup
    under aliases_mutex; the previous of_mutex path could race with
    the reconfig notifier that mutates under aliases_mutex.
  - Drop the defensive ATTACH_NODE property scan. Overlay attach
    queues each property as a separate ADD_PROPERTY changeset entry,
    so per-property notifications cover the case that matters;
    walking rd->dn->properties without devtree_lock could race with
    concurrent DT mutations. A direct of_attach_node() caller that
    pre-populates /aliases loses tracking for those entries, matching
    pre-series behavior.
  - Replace DETACH_NODE's per-property forget with a single scan of
    aliases_lookup itself (same rationale). Boot-time entries are
    unlinked; memblock-backed struct + alias name are not freed.
  - of_alias_destroy() now of_node_puts() the target for every
    matching entry, closing a leak of the reference
    of_find_node_by_path() took at of_alias_scan() time for boot-time
    entries.

Patch 3:
  - Enforce non-empty + null-terminated-within-pp->length up-front
    for every /aliases property value. Values that don't match
    "/fragment@" now flow through __of_prop_dup() only after that
    validation, closing a pre-existing hole where a malformed alias
    value could reach the live tree.

Patch 4:
  - Hold a reference on the grafted target node across the revert
    so the post-remove assertion queries the right np, not the
    base. of_alias_get_id(base, ...) would trivially return -ENODEV
    regardless of whether removal actually cleaned up the alias.
  - On of_overlay_fdt_apply() failure, reclaim the (possibly
    partially-populated) ovcs_id via of_overlay_remove() to avoid
    leaking the unflattened FDT and partially-applied nodes.

Additional fixes from local review of the v3 candidate:

All patches:
  - Trim inline comments to the constraint being enforced; rationale
    and use-case narrative moved to the commit messages.

Patch 1:
  - of_alias_create() releases the target-node reference on every
    error path, not only for owned entries — restoring the original
    of_alias_scan() error behavior for boot-time entries.
  - Correct the struct alias_prop @owned kernel-doc: every entry holds
    a reference on @np (removal drops it regardless of @owned); the
    flag only selects whether the struct and alias name are kfree()d.
  - Correct the notifier-registration comment and commit message:
    of_alias_scan() runs pre-initcall from unflatten_device_tree() /
    of_pdt_build_devicetree(), not from of_core_init().
  - DETACH_NODE clears the of_aliases pointer but keeps the reference
    taken at ATTACH_NODE: of_find_node_opts_by_path() reads of_aliases
    and walks its properties locklessly, so the node must outlive any
    concurrent reader.
  - Call out (in the commit message) the pre-existing one-byte OOB
    read in the stem parser that the refactor fixes: isdigit() was
    tested before the end > start bound.

Patch 3:
  - Correct the commit-message claim that a NULL return from
    dup_and_fixup_symbol_prop() can only mean -ENOMEM; unresolvable
    fragment paths also return NULL and are reported as -ENOMEM,
    matching the existing /__symbols__ handling.

Patch 4:
  - Fix the .dtso label: dtc labels cannot contain hyphens, so
    testcase-target failed to compile. Now testcase_target.
  - Do not add overlay_alias.dtbo to the fdtoverlay static build
    test: fdtoverlay cannot resolve the empty (base-relative)
    target-path, so static_test_1.dtb would fail to build.
  - Drop the unnecessary #address-cells/#size-cells from the grafted
    node (dtc W=1 avoid_unnecessary_addr_size).

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

Addressed 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.
    (v3 pared this back — see above.)
  - 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. Other alias
    values pass through the raw duplicator unchanged. This removes
    the previous unconditional fallback that would silently propagate
    a raw dup on ENOMEM.

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 and assert against the grafted node's np.

Link to v1: https://patch.msgid.link/20260720-nh-of-alias-overlay-v1-0-27da6848dd84@nexthop.ai

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

None outstanding. Pre-existing issues surfaced by the automated review
are unrelated to this series and are being tracked separately:
  - fragment target/overlay refcount leak on init_overlay_changeset()
    error paths (ovcs->count still 0 when free_overlay_changeset()
    runs),
  - deadprops double-free in add_changeset_property() when
    of_changeset_add_property() fails after the property was linked
    into target->np->deadprops,
  - of_node_put() under devtree_lock (raw spinlock) reaching
    fwnode_links_purge(),
  - lan966x_pci_probe() not reclaiming a populated ovcs_id when
    of_overlay_fdt_apply() fails.

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.

The unittest was verified on the same platform with OF_UNITTEST=y:
of_unittest_overlay_alias() passes; the full run reports 426 passed,
2 failed, both environmental (of_unittest_pci_node() requires QEMU's
pci-testdev, and of_unittest_overlay_gpio()'s probe-count assertion
trips over unrelated deferred probes on this PCI-heavy x86 box).

[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                           | 200 ++++++++++++++++++++++------
 drivers/of/device.c                         |   4 +-
 drivers/of/of_private.h                     |   7 +
 drivers/of/overlay.c                        |  41 +++---
 drivers/of/unittest-data/Makefile           |   5 +
 drivers/of/unittest-data/overlay_alias.dtso |  25 ++++
 drivers/of/unittest.c                       |  64 +++++++++
 7 files changed, 286 insertions(+), 60 deletions(-)
---
base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
change-id: 20260719-nh-of-alias-overlay-6e5e0d56212b

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/4] of: incrementally update /aliases lookup on reconfig notifications
  2026-07-21 21:36 [PATCH v3 0/4] of: teach overlay code to keep /aliases in sync Abdurrahman Hussain
@ 2026-07-21 21:36 ` Abdurrahman Hussain
  2026-07-21 21:48   ` sashiko-bot
  2026-07-21 21:36 ` [PATCH v3 2/4] of/overlay: look up absolute target-paths absolutely Abdurrahman Hussain
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Abdurrahman Hussain @ 2026-07-21 21:36 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan
  Cc: devicetree, linux-kernel, 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 that mirrors
/aliases changes into aliases_lookup. Registration happens at
core_initcall_sync time, safely after the boot-time of_alias_scan(),
which runs pre-initcall from unflatten_device_tree() (or
of_pdt_build_devicetree() on OF-real platforms):

  OF_RECONFIG_ADD_PROPERTY    -> of_alias_create()
  OF_RECONFIG_REMOVE_PROPERTY -> of_alias_destroy()
  OF_RECONFIG_UPDATE_PROPERTY -> destroy + create
  OF_RECONFIG_ATTACH_NODE     -> adopt the node as of_aliases
  OF_RECONFIG_DETACH_NODE     -> drop every aliases_lookup entry

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 handler takes a
reference on the adopted node that is deliberately never dropped:
of_find_node_opts_by_path() reads of_aliases and walks its property
list locklessly, so the node must stay valid even after DETACH clears
the pointer. Only per-property notifications populate aliases_lookup —
the notifier does not walk the attached node's property list, which
would race with devtree_lock-protected property mutations. A direct
of_attach_node() caller that pre-populates /aliases is not tracked,
matching pre-series behavior.

DETACH_NODE conversely drops every aliases_lookup entry: the only
/aliases node in the tree is going away, so every entry is backed by
it. Walking aliases_lookup itself (under aliases_mutex) avoids the
same property-list race. Overlay revert additionally emits per-
property REMOVE events beforehand; the sweep catches direct
of_detach_node() callers that don't.

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
so the alias_prop survives the property that spawned it — required
for the overlay revert path where the source property is freed. 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. Every entry, boot-time or runtime, holds the target-node
reference that of_find_node_by_path() returned at create time;
destroy drops it symmetrically.

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
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, of_device_uevent) and the
reconfig notifier 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.

The refactor also fixes a pre-existing one-byte out-of-bounds read in
the stem parser: the old loop tested isdigit(*(end - 1)) before
checking end > start, reading one byte before the property name when
the name is empty or all digits. of_alias_create() checks the bound
first and rejects a zero-length stem.

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       | 200 ++++++++++++++++++++++++++++++++++++++----------
 drivers/of/device.c     |   4 +-
 drivers/of/of_private.h |   7 ++
 3 files changed, 169 insertions(+), 42 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 6e7a42dedad3..4e34c65a8f9f 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1915,6 +1915,160 @@ static void of_alias_add(struct alias_prop *ap, struct device_node *np,
 		 ap->alias, ap->stem, ap->id, np);
 }
 
+/*
+ * Protects aliases_lookup and of_aliases. of_alias_scan() runs single-
+ * threaded at init and skips it; every other reader/writer must hold it.
+ */
+DEFINE_MUTEX(aliases_mutex);
+
+/* Callers other than of_alias_scan() must hold @aliases_mutex. */
+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;
+
+	/* of_find_node_by_path() derefs the value as a C string */
+	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);
+
+	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:
+	of_node_put(np);
+}
+
+/* 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);
+		of_node_put(ap->np);
+		if (ap->owned) {
+			kfree(ap->alias);
+			kfree(ap);
+		}
+		return;
+	}
+}
+
+static void *alias_alloc(u64 size, u64 align)
+{
+	return kzalloc(size, GFP_KERNEL);
+}
+
+/* Callers must hold @aliases_mutex. */
+static void of_aliases_forget_all(void)
+{
+	struct alias_prop *ap, *tmp;
+
+	list_for_each_entry_safe(ap, tmp, &aliases_lookup, link) {
+		list_del(&ap->link);
+		of_node_put(ap->np);
+		if (ap->owned) {
+			kfree(ap->alias);
+			kfree(ap);
+		}
+	}
+}
+
+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 || !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:
+		if (!of_aliases)
+			of_aliases = of_node_get(rd->dn);
+		break;
+	case OF_RECONFIG_DETACH_NODE:
+		of_aliases_forget_all();
+		/* keep the ATTACH reference: lockless readers may hold the 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:
+		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 pre-initcall, so the boot-time scan is complete */
+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 +2104,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 +2123,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 +2133,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 +2151,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 +2159,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/device.c b/drivers/of/device.c
index b3dc78f2fa3a..fa0cc8129ab0 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -237,7 +237,7 @@ void of_device_uevent(const struct device *dev, struct kobj_uevent_env *env)
 	add_uevent_var(env, "OF_COMPATIBLE_N=%d", seen);
 
 	seen = 0;
-	mutex_lock(&of_mutex);
+	mutex_lock(&aliases_mutex);
 	list_for_each_entry(app, &aliases_lookup, link) {
 		if (dev->of_node == app->np) {
 			add_uevent_var(env, "OF_ALIAS_%d=%s", seen,
@@ -245,7 +245,7 @@ void of_device_uevent(const struct device *dev, struct kobj_uevent_env *env)
 			seen++;
 		}
 	}
-	mutex_unlock(&of_mutex);
+	mutex_unlock(&aliases_mutex);
 }
 EXPORT_SYMBOL_GPL(of_device_uevent);
 
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 0ae16da066e2..3ed4ad64ff43 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -17,6 +17,11 @@
  * @alias:	Alias property name
  * @np:		Pointer to device_node that the alias stands for
  * @id:		Index value from end of alias name
+ * @owned:	True for runtime entries, where the struct and @alias are
+ *		kmalloc'd/kstrdup'd and freed on removal. False for boot-time
+ *		entries, which live in memblock (@alias points into the FDT)
+ *		and are only unlinked. Every entry holds a reference on @np;
+ *		removal drops it regardless of @owned.
  * @stem:	Alias string without the index
  *
  * The structure represents one alias property of 'aliases' node as
@@ -27,6 +32,7 @@ struct alias_prop {
 	const char *alias;
 	struct device_node *np;
 	int id;
+	bool owned;
 	char stem[];
 };
 
@@ -40,6 +46,7 @@ struct alias_prop {
 
 extern struct mutex of_mutex;
 extern raw_spinlock_t devtree_lock;
+extern struct mutex aliases_mutex;
 extern struct list_head aliases_lookup;
 extern struct kset *of_kset;
 

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/4] of/overlay: look up absolute target-paths absolutely
  2026-07-21 21:36 [PATCH v3 0/4] of: teach overlay code to keep /aliases in sync Abdurrahman Hussain
  2026-07-21 21:36 ` [PATCH v3 1/4] of: incrementally update /aliases lookup on reconfig notifications Abdurrahman Hussain
@ 2026-07-21 21:36 ` Abdurrahman Hussain
  2026-07-21 21:36 ` [PATCH v3 3/4] of/overlay: rewrite /aliases path values to live-tree paths Abdurrahman Hussain
  2026-07-21 21:36 ` [PATCH v3 4/4] of: unittest: cover /aliases updates from overlay apply/revert Abdurrahman Hussain
  3 siblings, 0 replies; 8+ messages in thread
From: Abdurrahman Hussain @ 2026-07-21 21:36 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan
  Cc: devicetree, linux-kernel, 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 | 26 ++++++++------------------
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 08d5351746be..b6545905cf67 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,14 @@ 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);
-			}
-		}
+		/* an empty target-path means the target base itself */
+		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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 3/4] of/overlay: rewrite /aliases path values to live-tree paths
  2026-07-21 21:36 [PATCH v3 0/4] of: teach overlay code to keep /aliases in sync Abdurrahman Hussain
  2026-07-21 21:36 ` [PATCH v3 1/4] of: incrementally update /aliases lookup on reconfig notifications Abdurrahman Hussain
  2026-07-21 21:36 ` [PATCH v3 2/4] of/overlay: look up absolute target-paths absolutely Abdurrahman Hussain
@ 2026-07-21 21:36 ` Abdurrahman Hussain
  2026-07-21 21:52   ` sashiko-bot
  2026-07-21 21:36 ` [PATCH v3 4/4] of: unittest: cover /aliases updates from overlay apply/revert Abdurrahman Hussain
  3 siblings, 1 reply; 8+ messages in thread
From: Abdurrahman Hussain @ 2026-07-21 21:36 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan
  Cc: devicetree, linux-kernel, 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, reject values that aren't non-empty C strings
null-terminated within pp->length (of_alias_get_id() and
of_find_node_by_path() dereference alias values as C strings, so a
malformed one must not reach the live tree through either dup path),
and then route values with a "/fragment@" prefix through
dup_and_fixup_symbol_prop(). 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 instead of falling back to a raw dup when
dup_and_fixup_symbol_prop() returns NULL matters: a fallback would
inject the unrewritten fragment path into the live tree where nothing
can resolve it. With the string shape validated here, a NULL return
means either allocation failure or a fragment path that doesn't
resolve within the overlay; both are reported as -ENOMEM, matching
the existing /__symbols__ handling of the same helper. Distinguishing
the two would require an ERR_PTR conversion of the shared helper and
is left for a separate cleanup.

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

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index b6545905cf67..9f85278c577a 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -350,6 +350,21 @@ 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")) {
+		/* /aliases values are deref'd as C strings; reject malformed input */
+		if (!overlay_prop->value || overlay_prop->length < 2 ||
+		    strnlen(overlay_prop->value, overlay_prop->length) >=
+			overlay_prop->length)
+			return -EINVAL;
+
+		/* rewrite overlay-internal paths to live-tree paths */
+		if (!strncmp(overlay_prop->value, "/fragment@",
+			     strlen("/fragment@")))
+			new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop);
+		else
+			new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL);
 	} else {
 		new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL);
 	}

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 4/4] of: unittest: cover /aliases updates from overlay apply/revert
  2026-07-21 21:36 [PATCH v3 0/4] of: teach overlay code to keep /aliases in sync Abdurrahman Hussain
                   ` (2 preceding siblings ...)
  2026-07-21 21:36 ` [PATCH v3 3/4] of/overlay: rewrite /aliases path values to live-tree paths Abdurrahman Hussain
@ 2026-07-21 21:36 ` Abdurrahman Hussain
  2026-07-21 21:46   ` sashiko-bot
  3 siblings, 1 reply; 8+ messages in thread
From: Abdurrahman Hussain @ 2026-07-21 21:36 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan
  Cc: devicetree, linux-kernel, 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.

The overlay is not added to the fdtoverlay static build test:
fdtoverlay has no target-base concept, so fragment@0's empty
target-path (meaning "the target base itself" at run time) cannot be
resolved statically.

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

diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
index 01a966e39f23..fdfec5a7923b 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 \
@@ -66,6 +67,10 @@ DTC_FLAGS_testcases += -Wno-interrupts_property \
 #			  overlay_bad_add_dup_prop.dtbo \
 #			  overlay_bad_phandle.dtbo \
 #			  overlay_bad_symbol.dtbo \
+#
+# overlay_alias.dtbo needs a run-time target base for its empty target-path;
+# fdtoverlay has no equivalent, so it is also not included:
+#			  overlay_alias.dtbo \
 
 apply_static_overlay_1 := overlay_0.dtbo \
 			  overlay_1.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..17e02d1940f6
--- /dev/null
+++ b/drivers/of/unittest-data/overlay_alias.dtso
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+/*
+ * overlay_alias - declare an /aliases entry that points, via label, at a
+ * node grafted under the run-time target base (empty target-path).
+ */
+
+/ {
+	fragment@0 {
+		target-path = "";
+		__overlay__ {
+			testcase_target: alias-target {
+			};
+		};
+	};
+
+	fragment@1 {
+		target-path = "/aliases";
+		__overlay__ {
+			testcase-alias99 = &testcase_target;
+		};
+	};
+};
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index e255f54f4d76..308daadf22c8 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -3475,6 +3475,68 @@ static struct notifier_block of_nb = {
 	.notifier_call = of_notify,
 };
 
+/* created by cmd_wrap_S_dtbo in scripts/Makefile.dtbs */
+extern uint8_t __dtbo_overlay_alias_begin[];
+extern uint8_t __dtbo_overlay_alias_end[];
+
+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";
+	u32 size = __dtbo_overlay_alias_end - __dtbo_overlay_alias_begin;
+	struct device_node *base, *target = NULL;
+	int ovcs_id = 0;
+	int id, ret;
+
+	base = of_find_node_by_path(base_path);
+	if (!base) {
+		unittest(0, "could not find alias target base %s\n", base_path);
+		return;
+	}
+
+	ret = of_overlay_fdt_apply(__dtbo_overlay_alias_begin, size,
+				   &ovcs_id, base);
+	if (ret < 0) {
+		unittest(0, "overlay_alias apply failed, ret = %d\n", ret);
+		if (ovcs_id)
+			of_overlay_remove(&ovcs_id);
+		goto out_put_base;
+	}
+
+	/* hold a ref across the revert for the post-remove check */
+	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);
+
+out_remove:
+	ret = of_overlay_remove(&ovcs_id);
+	if (ret) {
+		unittest(0, "overlay_alias remove failed, ret = %d\n", ret);
+		of_node_put(target);
+		goto out_put_base;
+	}
+
+	if (target) {
+		id = of_alias_get_id(target, "testcase-alias");
+		unittest(id == -ENODEV,
+			 "of_alias_get_id() = %d after overlay remove, expected -ENODEV\n",
+			 id);
+		of_node_put(target);
+	}
+
+out_put_base:
+	of_node_put(base);
+}
+
 static void __init of_unittest_overlay_notify(void)
 {
 	int ovcs_id;
@@ -3649,6 +3711,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();

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 4/4] of: unittest: cover /aliases updates from overlay apply/revert
  2026-07-21 21:36 ` [PATCH v3 4/4] of: unittest: cover /aliases updates from overlay apply/revert Abdurrahman Hussain
@ 2026-07-21 21:46   ` sashiko-bot
  0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-07-21 21:46 UTC (permalink / raw)
  To: Abdurrahman Hussain; +Cc: robh, conor+dt, devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Holding a device node reference across `of_overlay_remove()` causes a memory leak and unexpected kernel error logs.
--

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

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

This commit adds an overlay test to ensure /aliases updates work correctly
across apply and revert when the overlay grafts a node with an alias.

> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c

[ ... ]

> +static void __init of_unittest_overlay_alias(void)
> +{

[ ... ]

> +	/* hold a ref across the revert for the post-remove check */
> +	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);
> +
> +out_remove:
> +	ret = of_overlay_remove(&ovcs_id);

[Severity: Medium]
Does holding this device node reference across of_overlay_remove() cause a
memory leak and unexpected kernel error logs?

When the reference to the target node is held, __of_changeset_entry_destroy()
observes a refcount greater than 1, which triggers an "ERROR: memory leak"
log and skips the OF_OVERLAY_FREE_CSET flag.

Subsequently, when of_node_put(target) drops the refcount down to 0,
of_node_release() logs another memory leak error and returns without freeing
the node memory.

> +	if (ret) {
> +		unittest(0, "overlay_alias remove failed, ret = %d\n", ret);
> +		of_node_put(target);
> +		goto out_put_base;
> +	}
> +
> +	if (target) {
> +		id = of_alias_get_id(target, "testcase-alias");
> +		unittest(id == -ENODEV,
> +			 "of_alias_get_id() = %d after overlay remove, expected -ENODEV\n",
> +			 id);
> +		of_node_put(target);
> +	}
> +
> +out_put_base:
> +	of_node_put(base);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-nh-of-alias-overlay-v3-0-7001028fe2f5@nexthop.ai?part=4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/4] of: incrementally update /aliases lookup on reconfig notifications
  2026-07-21 21:36 ` [PATCH v3 1/4] of: incrementally update /aliases lookup on reconfig notifications Abdurrahman Hussain
@ 2026-07-21 21:48   ` sashiko-bot
  0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-07-21 21:48 UTC (permalink / raw)
  To: Abdurrahman Hussain; +Cc: devicetree, robh, conor+dt

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The deliberate memory leak of the `/aliases` node reference during `OF_RECONFIG_DETACH_NODE` breaks overlay revert and causes an unbounded memory leak.
--

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

of: incrementally update /aliases lookup on reconfig notifications

This commit adds an OF reconfig notifier to dynamically update the
/aliases lookup list when nodes or properties change. It handles both
direct devicetree modifications and overlay apply/revert paths, protecting
access to the list with a dedicated aliases_mutex.

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..4e34c65a8f9f0 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 || !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:
> +		if (!of_aliases)
> +			of_aliases = of_node_get(rd->dn);
> +		break;
> +	case OF_RECONFIG_DETACH_NODE:
> +		of_aliases_forget_all();
> +		/* keep the ATTACH reference: lockless readers may hold the node */
> +		if (of_aliases == rd->dn)
> +			of_aliases = NULL;

[Severity: High]
Does skipping of_node_put() here cause a memory leak of the device node
and its properties when reverting overlays?

During the ATTACH_NODE notification, the notifier takes a reference on the
node using of_node_get(). When the overlay is later reverted, the DETACH_NODE
handler clears the pointer but intentionally omits of_node_put().

When the overlay changeset is subsequently destroyed,
__of_changeset_entry_destroy() checks the node's reference count. Because this
notifier leaked a reference, the count remains elevated. 

This elevated count causes the destruction process to print a memory leak error
and skip setting the OF_OVERLAY_FREE_CSET flag.

Consequently, of_node_release() will refuse to free the node's memory, leaking
the node on every overlay apply/revert cycle.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-nh-of-alias-overlay-v3-0-7001028fe2f5@nexthop.ai?part=1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 3/4] of/overlay: rewrite /aliases path values to live-tree paths
  2026-07-21 21:36 ` [PATCH v3 3/4] of/overlay: rewrite /aliases path values to live-tree paths Abdurrahman Hussain
@ 2026-07-21 21:52   ` sashiko-bot
  0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-07-21 21:52 UTC (permalink / raw)
  To: Abdurrahman Hussain; +Cc: conor+dt, devicetree, robh

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

New issues:
- [Medium] Pseudo-properties of newly created /aliases nodes are incorrectly validated as C string paths, breaking valid overlays.

Pre-existing issues:
- [High] Use-After-Free and Double Free in add_changeset_property error path.
- [Medium] Memory leak of duplicated device node on error path in add_changeset_node().
--

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

of/overlay: rewrite /aliases path values to live-tree paths

This commit rewrites overlay-internal path strings for /aliases entries to
live-tree paths, similar to /__symbols__. It ensures that /aliases values
correctly reference nodes in the live tree.

> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index b6545905cf67d..9f85278c577ac 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -350,6 +350,21 @@ 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")) {
> +		/* /aliases values are deref'd as C strings; reject malformed input */
> +		if (!overlay_prop->value || overlay_prop->length < 2 ||
> +		    strnlen(overlay_prop->value, overlay_prop->length) >=
> +			overlay_prop->length)
> +			return -EINVAL;

[Severity: Medium]
Does this code incorrectly reject pseudo-properties on newly created /aliases
nodes?

Since the is_pseudo_property() filter at the top of add_changeset_property()
only applies when target->in_livetree is true, pseudo-properties (like
"phandle") on a newly created /aliases node will fall through to this logic.

A "phandle" is a 32-bit integer, so it may lack a null byte. This causes
strnlen() >= length to evaluate to true, falsely returning -EINVAL and aborting
valid overlay applications.

> +
> +		/* rewrite overlay-internal paths to live-tree paths */
> +		if (!strncmp(overlay_prop->value, "/fragment@",
> +			     strlen("/fragment@")))
> +			new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop);
> +		else
> +			new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL);
>  	} else {
>  		new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL);
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but does the error path in
add_changeset_property() cause a use-after-free or double-free?

If of_changeset_add_property() fails later in this function, we do this:

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);
	} else {
    ...
	if (ret)
		__of_prop_free(new_prop);
	return ret;
}

Since new_prop is freed but left linked in target->np->deadprops, won't
of_node_release() later iterate over deadprops and double-free this same
property?


[Severity: Medium]
This isn't a bug introduced by this patch, but does the error path in
add_changeset_node() permanently leak the duplicated node tchild?

When attaching a newly duplicated child node fails, the function returns
immediately without freeing the node:

drivers/of/overlay.c:add_changeset_node() {
    ...
	if (!tchild) {
		tchild = __of_node_dup(NULL, node_kbasename);
        ...
		ret = of_changeset_attach_node(&ovcs->cset, tchild);
		if (ret)
			return ret;
    ...
}

Should there be an of_node_put(tchild) before returning the error?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-nh-of-alias-overlay-v3-0-7001028fe2f5@nexthop.ai?part=3

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-07-21 21:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-21 21:36 [PATCH v3 0/4] of: teach overlay code to keep /aliases in sync Abdurrahman Hussain
2026-07-21 21:36 ` [PATCH v3 1/4] of: incrementally update /aliases lookup on reconfig notifications Abdurrahman Hussain
2026-07-21 21:48   ` sashiko-bot
2026-07-21 21:36 ` [PATCH v3 2/4] of/overlay: look up absolute target-paths absolutely Abdurrahman Hussain
2026-07-21 21:36 ` [PATCH v3 3/4] of/overlay: rewrite /aliases path values to live-tree paths Abdurrahman Hussain
2026-07-21 21:52   ` sashiko-bot
2026-07-21 21:36 ` [PATCH v3 4/4] of: unittest: cover /aliases updates from overlay apply/revert Abdurrahman Hussain
2026-07-21 21:46   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox