* [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