* [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
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ 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] 9+ 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; 9+ 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] 9+ 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
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ 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] 9+ 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
2026-08-19 11:49 ` [PATCH v3 0/4] of: teach overlay code to keep /aliases in sync Herve Codina
4 siblings, 1 reply; 9+ 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] 9+ 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; 9+ 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] 9+ 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
2026-08-19 11:49 ` [PATCH v3 0/4] of: teach overlay code to keep /aliases in sync Herve Codina
4 siblings, 1 reply; 9+ 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] 9+ 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; 9+ 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] 9+ messages in thread
* Re: [PATCH v3 0/4] of: teach overlay code to keep /aliases in sync
2026-07-21 21:36 [PATCH v3 0/4] of: teach overlay code to keep /aliases in sync Abdurrahman Hussain
` (3 preceding siblings ...)
2026-07-21 21:36 ` [PATCH v3 4/4] of: unittest: cover /aliases updates from overlay apply/revert Abdurrahman Hussain
@ 2026-08-19 11:49 ` Herve Codina
4 siblings, 0 replies; 9+ messages in thread
From: Herve Codina @ 2026-08-19 11:49 UTC (permalink / raw)
To: Abdurrahman Hussain
Cc: Rob Herring, Saravana Kannan, devicetree, linux-kernel
Hi Abdurrahman,
On Tue, 21 Jul 2026 14:36:31 -0700
Abdurrahman Hussain <abdurrahman@nexthop.ai> wrote:
> /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.
Are you sure aliases were mentioned in the talk and were the cause of issues?
I've got an issue with i2c muxes but it is not related to aliases.
What made you think about aliases issue?
The issue I have is related to fw_devlink consumer/supplier relationship and
I have sent a fix [0] with nothing related to aliases.
[0] https://lore.kernel.org/all/20260630103010.413688-1-herve.codina@bootlin.com/
Best regards,
Hervé
^ permalink raw reply [flat|nested] 9+ messages in thread