[PATCH v7 00/10] of: teach overlay code to keep /aliases in sync

[PATCH v7 00/10] of: teach overlay code to keep /aliases in sync

[Abdurrahman Hussain]

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

The gap has been public since 2015 [1] and reproduces trivially: apply
an overlay that declares e.g. `i2c99 = &foo;`, ask
of_alias_get_id(foo, "i2c") -> -ENODEV.

The core fix (patch 2) is a reconfig notifier that mirrors /aliases
property changes into aliases_lookup. Patch 1 prepares the alias path
lookup for /aliases becoming dynamic. Patches 3, 8 and 9 are the
overlay-code changes the use case needs; patches 4-7 fix pre-existing
bugs on paths the series exercises. Patch 10 adds a
unittest.

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 makes of_find_node_opts_by_path() hold a reference on
of_aliases across the alias walk (the walk itself stays lock-free, as
does the pointer load: of_aliases always holds a reference on the
node it points to) and validates alias values before dereferencing
them.

Patch 2 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 3 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 4 fixes a pre-existing double-free in add_changeset_property():
a property freed on the of_changeset_add_property() error path stayed
linked in the target node's deadprops list and was freed again at
node release.

Patch 5 fixes a pre-existing NULL dereference in
of_overlay_fdt_apply()'s idr_alloc() error path (negative id stored,
list_del() on an uninitialized list head) by only treating a positive
id as registered in free_overlay_changeset().

Patch 6 fixes a pre-existing reference leak in
init_overlay_changeset(): on a mid-loop failure, the target and
overlay references of the fragments initialized so far were never
dropped because ovcs->count was still 0 when free_overlay_changeset()
ran its cleanup loop.

Patch 7 fixes pre-existing "//" result paths from
dup_and_fixup_symbol_prop() when a fragment targets the root node.

Patch 8 converts dup_and_fixup_symbol_prop() to return ERR_PTR so
callers can tell malformed values, not-overlay-internal paths, and
allocation failures apart — /__symbols__ errors stop being reported
as -ENOMEM regardless of cause — and makes it verify the value
actually descends through the matched fragment's __overlay__ node
before slicing, instead of cutting at an assumed prefix length.

Patch 9 rewrites /aliases property values from the overlay's internal
fragment path ("/<fragment>/__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 2's notifier receives paths that never resolve in the
live tree, so of_alias_get_id() still returns -ENODEV. Values that
don't resolve inside the overlay (legacy string aliases, malformed
junk) are copied verbatim and stay inert — consumers validate before
dereferencing — so no overlay that applied before this series stops
applying.

Patch 10 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() on the grafted node returns 99 after the
apply and of_alias_get_highest_id() reports the stem gone after the
revert. Missing any one of the three functional patches (notifier /
absolute-target-paths / alias path rewrite) causes the assertion to
fail.

Changes in v7
-------------
Addresses Rob's review of v6 and an automated-review finding
acknowledged on the list. Patch numbers below are v7's; v6's patch 1
(the stem-parser out-of-bounds fix) is applied and dropped, so
former patches 2-6 shift down by one.

Dropped former patch 1:
  - Applied to Rob's dt/linus as commit 5bb01c657ff9. The series is
    rebased onto it, since patch 2 moves the fixed parser loop into
    of_alias_create().

Patch 1 (was 2, Rob):
  - The of_aliases pointer load is a plain of_node_get() again;
    devtree_lock added nothing since of_aliases always holds a
    reference on the node it points to.

Patch 2 (was 3):
  - Consequently, the reconfig notifier no longer takes devtree_lock
    around the of_aliases updates either; writers were already
    serialized by aliases_mutex.

New patch 6:
  - Fix a pre-existing reference leak in init_overlay_changeset()'s
    error path, found by Sashiko AI review on v6.

Tags:
  - Patch 1 gains Fixes: c22e650e66b8 ("of: Make
    of_find_node_by_path() handle /aliases") for its validation half.
  - Patch 4 gains Fixes: f96278810150 ("of: overlay: set node fields
    from properties when add new overlay node") and Cc: stable, in
    line with the other pre-existing-bug fixes in the series.

Cover letter:
  - Dropped the reference to Bootlin's ELCE 2025 PCI-overlay talk: it
    described a separate fw_devlink consumer/supplier issue, not the
    aliases gap this series fixes (Hervé Codina).

Link to v6: https://patch.msgid.link/20260805-nh-of-alias-overlay-v6-0-74f21d440819@nexthop.ai

Changes in v6
-------------
Addresses Geert's review of v5 and two automated-review findings the
list replies already acknowledged:

New patch 1 (Geert):
  - The out-of-bounds read fix in the stem parser is split out of the
    notifier patch as a standalone, backportable fix.

Patch 2:
  - Corrected the lock-free-walk description: a racing walk can see a
    stale view (a removed property's ->next points into deadprops);
    what the node reference guarantees is that nothing reachable is
    freed. The code is unchanged.

Patch 6 (Geert):
  - The guard moved into free_overlay_changeset(): only a
    strict-positive id is treated as registered, replacing the
    ovcs->id reset at the error site. (A plain "goto out_unlock"
    would leak the ovcs allocation; free_overlay_changeset() is safe
    to call on a partially initialized ovcs.)

Patch 7 (was patch 6):
  - Keep the "/" target path when the rewritten value names the
    fragment root itself; dropping it unconditionally produced an
    empty string for that case.

Link to v5: https://patch.msgid.link/20260722-nh-of-alias-overlay-v5-0-2abe2bb9cdbc@nexthop.ai

Changes in v5
-------------

Numbering note: the 7-patch series posted on 2026-07-22 went out
mislabeled as a second v4 (a tooling mistake on my side). This
posting supersedes both v4 threads.

Addresses Krzysztof's review of the first v4 posting and the
automated review of the second:

Patch 1 (Krzysztof):
  - The property walk is lock-free again; devtree_lock covers only
    the of_aliases pointer load and of_node_get(). Removed properties
    stay on deadprops with ->next intact, so the node reference is
    what a walker needs — the loop never needed the lock. Retitled
    accordingly.
  - The commit message no longer refers to overlay-writable /aliases
    in the present tense at a point in the series where it isn't.

All patches (Krzysztof):
  - Commit messages rewritten and shortened.

New patch 5:
  - Fix a pre-existing NULL dereference in of_overlay_fdt_apply():
    a failed idr_alloc() left a negative id in ovcs->id, so
    free_overlay_changeset() ran idr_remove() with a negative id and
    list_del() on an uninitialized list head.

New patch 6:
  - Fix pre-existing "//" paths from dup_and_fixup_symbol_prop()
    when a fragment targets the root node; the malformed path made
    symbols (and now aliases) in such fragments unresolvable.

Link to the mislabeled second v4: https://patch.msgid.link/20260722-nh-of-alias-overlay-v4-0-fc96a40d2761@nexthop.ai

Changes in the second v4 posting (2026-07-22, 7 patches)
--------------------------------------------------------
Addresses the automated review of the first v4 posting (its patches
2, 3, and 6 drew no findings).

Patch 1:
  - of_alias_value_ok() additionally requires the value to be an
    absolute path (per the DT spec). A relative value naming another
    alias — including itself — sent of_find_node_by_path() into
    unbounded mutual recursion with alias resolution and exhausted
    the kernel stack; the shared helper closes it for the path
    lookup and of_alias_create() alike.

New patch 4:
  - Fix the pre-existing double-free in add_changeset_property():
    a property freed on the of_changeset_add_property() error path
    stayed linked in the target node's deadprops list and was freed
    again by of_node_release(). The property now goes onto deadprops
    only after the changeset entry is recorded, so the error path
    frees an unreferenced property. Flagged on every revision of
    this series; fixing it beats re-deferring it.

Patch 5 (was patch 4):
  - dup_and_fixup_symbol_prop() now verifies the value textually
    descends through the matched fragment's __overlay__ node before
    slicing. Previously only the first path component was resolved,
    so an absolute live-tree alias sharing its first component with
    a hand-named fragment (or a bogus /__symbols__ value) was sliced
    mid-string and rewritten to garbage. Prefix mismatch and
    too-short values are -ENODEV (not overlay-internal), no longer
    -EINVAL.

Link to v4: https://patch.msgid.link/20260721-nh-of-alias-overlay-v4-0-8ad097e31e36@nexthop.ai

Changes in v4
-------------
Addresses the automated review of v3, plus findings from a local
adversarial review pass over the v4 candidate.

New patch 1:
  - of_find_node_opts_by_path() walked of_aliases' property list with
    no lock and no node reference. Racy against devtree_lock-protected
    property surgery since the walk was introduced, but only reachable
    in practice once overlays can create and destroy /aliases.
    Prerequisite for the DETACH change below.
  - The reader also validates the value's shape (of_alias_value_ok())
    before dereferencing it as a C string — raw of_add_property()
    producers are not validated anywhere — and no longer passes the
    NULL value of an empty alias property to of_find_node_by_path().

Patch 2 (was patch 1):
  - DETACH_NODE now drops the reference taken at ATTACH_NODE after
    clearing the of_aliases pointer under devtree_lock. v3 kept the
    reference to protect lockless readers, but a kept reference
    violates the changeset-destroy contract —
    __of_changeset_entry_destroy() expects refcount 1 — so every
    apply/revert cycle of an /aliases-creating overlay logged
    "ERROR: memory leak" and permanently leaked the node. With the
    reader from patch 1 taking its own reference under devtree_lock,
    the put is safe.
  - The aliases_lookup sweep now runs only when the detached node is
    the tracked of_aliases, so a stray duplicate root node named
    "aliases" can no longer wipe entries backed by the real node; the
    node match is an exact-name compare (of_node_is_aliases()) since
    of_node_name_eq() would also match "aliases@1".
  - The notifier machinery is compiled only for CONFIG_OF_DYNAMIC;
    the of_reconfig_notifier_register() stub returns -EINVAL, so the
    unconditional core_initcall_sync failed on every non-dynamic DT
    kernel.
  - Per-entry teardown factored into __of_alias_del(); shared
    of_node_is_aliases()/of_alias_value_ok() helpers in of_private.h
    replace open-coded copies in base.c and overlay.c.

New patch 4:
  - dup_and_fixup_symbol_prop() returns ERR_PTR, distinguishing
    malformed values (-EINVAL), not-overlay-internal paths (-ENODEV),
    and allocation failure (-ENOMEM) — the discrimination v3's
    "/fragment@" prefix heuristic approximated, and the reason a
    structural failure was previously misreported as -ENOMEM.

Patch 5 (was patch 3):
  - Drop the "/fragment@" prefix heuristic: init_overlay_changeset()
    accepts fragments with any node name, so a hand-named fragment's
    alias would silently stay unrewritten (the bug this series fixes),
    and a live-tree path starting with "/fragment@" would fail apply.
    Discrimination now comes from where the value actually resolves
    (patch 4).
  - Malformed alias values no longer fail the overlay apply with
    -EINVAL; they are admitted verbatim with a warning and stay inert
    (consumers validate before dereferencing). An overlay that applied
    cleanly before this series keeps applying.
  - Exempt pseudo-properties from the /aliases handling: the
    is_pseudo_property() skip at the top of add_changeset_property()
    is gated on target->in_livetree, so a phandle (a raw cell, not a
    C string) of a newly created /aliases node could fail the string
    checks and abort a valid overlay.

Patch 6 (was patch 4):
  - Drop the grafted-node reference before of_overlay_remove();
    holding it across the revert trips the same changeset-destroy
    refcount check (kernel ERROR + leaked node).
  - Assert alias removal via of_alias_get_highest_id() — keyed by
    stem, needs no node reference — with a matching precondition
    check before the apply.
  - Wrap the apply in EXPECT_BEGIN/END for the standard "memory leak
    will occur" warning printed for properties added to a node the
    overlay didn't create.

Patch 3:
  - Document the empty-vs-absolute target-path contract in the
    kernel-doc for @base/@target_base and find_target(); the old text
    predated the semantic change.

Link to v3: https://patch.msgid.link/20260721-nh-of-alias-overlay-v3-0-7001028fe2f5@nexthop.ai

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),
  - node duplicated by __of_node_dup() leaked in add_changeset_node()
    when of_changeset_attach_node() fails,
  - of_node_put() under devtree_lock (raw spinlock) reaching
    fwnode_links_purge(),
  - of_overlay_fdt_apply() callers (lan966x_pci_probe(),
    tilcdc_panel_legacy_probe()) not reclaiming a populated ovcs_id
    on failure.

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

The OF unittest was run under QEMU (x86_64, OF_UNITTEST=y, v7.2-rc3
plus this series): of_unittest_overlay_alias() passes and the full
run reports 415 passed, 0 failed, with no unexpected "OF: ERROR:"
lines or changeset-destroy refcount complaints in the log. base.c
also builds with CONFIG_OF=y and CONFIG_OF_DYNAMIC=n (the notifier
machinery compiles out).

The series was also backported onto a 7.1-based image and boot-tested
on an x86 Nexthop switch with two PCI-attached Xilinx FPGAs whose
i2c-xiic controllers come from driver-embedded DT overlays applied
with a non-NULL target base. 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: 95 i2c
adapters with /dev/i2c-N numbering matching the overlay aliases
exactly and stable across reboots, sensors reading live telemetry
through mux channels, and dmesg free of WARN/BUG/Oops and unexpected
OF errors. Runtime overlay revert via driver unbind was exercised on
the same hardware: the changeset revert removed all 153 of the
overlay's /aliases entries through the reconfig notifier with no
refcount or of_node_put errors, leaving the other overlay's 12
entries intact. (The revert then hangs in device teardown, in a
pre-existing i2c-xiic i2c_del_adapter() issue unrelated to this
series; the complete apply/revert cycle is covered by the unittest
under QEMU.)

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

Assisted-by: Claude:claude-fable-5 [Claude Code]
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
Abdurrahman Hussain (10):
      of: hold a reference on of_aliases during alias path resolution
      of: update /aliases lookup on reconfig notifications
      of/overlay: look up absolute target-paths absolutely
      of/overlay: put property on deadprops only after changeset add succeeds
      of/overlay: only treat a positive changeset id as registered
      of/overlay: don't leak fragment references when changeset init fails
      of/overlay: don't create "//" paths for fragments targeting the root
      of/overlay: return ERR_PTR from dup_and_fixup_symbol_prop()
      of/overlay: rewrite /aliases path values to live-tree paths
      of: unittest: cover /aliases updates from overlay apply/revert

 drivers/of/base.c                           | 216 ++++++++++++++++++++++------
 drivers/of/device.c                         |   4 +-
 drivers/of/of_private.h                     |  22 +++
 drivers/of/overlay.c                        | 103 ++++++++-----
 drivers/of/unittest-data/Makefile           |   5 +
 drivers/of/unittest-data/overlay_alias.dtso |  25 ++++
 drivers/of/unittest.c                       |  73 ++++++++++
 7 files changed, 368 insertions(+), 80 deletions(-)
---
base-commit: 5bb01c657ff9fc807c2c592ca18af34c4fc3bc6f
change-id: 20260719-nh-of-alias-overlay-6e5e0d56212b

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

[PATCH v7 01/10] of: hold a reference on of_aliases during alias path resolution

[Abdurrahman Hussain]

of_find_node_opts_by_path() walks the property list of of_aliases
without taking a reference on the node and passes pp->value straight
to of_find_node_by_path().

Take a reference across the walk. The walk itself stays lock-free
like every other property iteration: it can race property surgery and
see a stale view (a removed property's ->next is repointed at the
deadprops list), but nothing it can reach is freed while the node
reference is held. The pointer load itself needs no locking:
of_aliases always holds a reference on the node it points to, so a
reader that observes a non-NULL pointer observes a live node. A later
patch in this series clears of_aliases and drops that reference when
the node is detached at runtime; the detach path holds its own
references across the transition.

Validate the value before resolving it. of_alias_value_ok() requires
a non-empty, NUL-terminated, absolute path:

  - an empty property has a NULL value and crashes in strchr()
  - a value without a NUL inside the property is read past its end
  - a relative value naming another alias (loop = "loop") recurses
    through of_find_node_by_path() until the stack is exhausted

All three are reachable with a malformed boot FDT today.

The name comparison loses its redundant strlen() pass while here.

Fixes: c22e650e66b8 ("of: Make of_find_node_by_path() handle /aliases")
Assisted-by: Claude:claude-fable-5 [Claude Code]
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/of/base.c       | 17 ++++++++++++-----
 drivers/of/of_private.h |  8 ++++++++
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 378703dbc11f..f7aa14d90e50 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -995,6 +995,8 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt
 
 	/* The path could begin with an alias */
 	if (*path != '/') {
+		struct device_node *aliases;
+		const char *value = NULL;
 		int len;
 		const char *p = strchrnul(path, '/');
 
@@ -1002,16 +1004,21 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt
 			p = separator;
 		len = p - path;
 
-		/* of_aliases must not be NULL */
-		if (!of_aliases)
+		aliases = of_node_get(of_aliases);
+		if (!aliases)
 			return NULL;
 
-		for_each_property_of_node(of_aliases, pp) {
-			if (strlen(pp->name) == len && !strncmp(pp->name, path, len)) {
-				np = of_find_node_by_path(pp->value);
+		for_each_property_of_node(aliases, pp) {
+			if (!strncmp(pp->name, path, len) && !pp->name[len]) {
+				if (of_alias_value_ok(pp))
+					value = pp->value;
 				break;
 			}
 		}
+		/* the reference on @aliases keeps @value alive */
+		if (value)
+			np = of_find_node_by_path(value);
+		of_node_put(aliases);
 		if (!np)
 			return NULL;
 		path = p;
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 0ae16da066e2..9bba999f0bf8 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -215,6 +215,14 @@ static inline bool is_pseudo_property(const char *prop_name)
 		!of_prop_cmp(prop_name, "linux,phandle");
 }
 
+/* alias values must be absolute paths NUL-terminated within length */
+static inline bool of_alias_value_ok(const struct property *pp)
+{
+	return pp->value && pp->length >= 2 &&
+	       *(const char *)pp->value == '/' &&
+	       strnlen(pp->value, pp->length) < pp->length;
+}
+
 #if IS_ENABLED(CONFIG_KUNIT)
 int __of_address_resource_bounds(struct resource *r, u64 start, u64 size);
 #endif

-- 
2.54.0

[PATCH v7 02/10] of: update /aliases lookup on reconfig notifications

[Abdurrahman Hussain]

Aliases added by overlays never make it into aliases_lookup, which is
only filled by of_alias_scan() at boot. of_alias_get_id() returns
-ENODEV for them and drivers using alias based numbering (i2c, spi,
tty, mmc) fall back to dynamic ids.

Register a reconfig notifier and mirror /aliases property changes
into aliases_lookup. The notifier chain covers changesets and
overlays, so no overlay specific hook is needed. Same approach as
Geert's 2015 series [1], which was never reposted;
of_alias_create()/of_alias_destroy() keep its names.

The /aliases node is matched by name and root parent instead of the
of_aliases pointer, which is still NULL when an overlay creates the
node on a system without a boot-time /aliases. The name match is
exact, of_node_name_eq() would also match "aliases@1". ATTACH stores
the node in of_aliases with a reference held, DETACH drops it again
and flushes aliases_lookup. The reference makes the lock-free
of_node_get(of_aliases) in of_find_node_opts_by_path() safe: a reader
that observes a non-NULL pointer observes a live node, and on DETACH
the changeset holds its own references across the notifier so the put
here cannot be the final one while readers still walk the node.
Nodes attached with properties already set are not scanned, as
before.

Lookup entries are created by of_alias_scan()'s old loop body, moved
into of_alias_create(). Entries created at runtime have kstrdup'ed
names and an of_node_get'ed target and are flagged "owned" so
of_alias_destroy() knows what to kfree(); boot entries live in
memblock and are only unlinked. Removal matches entries regardless of
ownership so updating a boot-time alias does not leave duplicates
behind, which was Grant's main concern on the old series [2].

A new aliases_mutex protects the list. of_mutex does not work here:
the notifier runs under it on the overlay path but outside of it on
the of_add_property() path.

of_alias_create() skips names that are empty or all digits instead of
creating an entry with an empty stem.

Only built for CONFIG_OF_DYNAMIC: there are no notifications without
it and the register stub returns -EINVAL.

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]
Assisted-by: Claude:claude-fable-5 [Claude Code]
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/of/base.c       | 199 ++++++++++++++++++++++++++++++++++++++----------
 drivers/of/device.c     |   4 +-
 drivers/of/of_private.h |  14 ++++
 3 files changed, 175 insertions(+), 42 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index f7aa14d90e50..23d9bb073d8b 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1926,6 +1926,159 @@ 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;
+
+	if (!of_alias_value_ok(pp))
+		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);
+}
+
+#ifdef CONFIG_OF_DYNAMIC
+/* Unlink @ap; free its storage if it was runtime-allocated. */
+static void __of_alias_del(struct alias_prop *ap)
+{
+	list_del(&ap->link);
+	of_node_put(ap->np);
+	if (ap->owned) {
+		kfree(ap->alias);
+		kfree(ap);
+	}
+}
+
+/* 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;
+		__of_alias_del(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)
+		__of_alias_del(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 || !of_node_is_aliases(rd->dn))
+		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:
+		if (of_aliases == rd->dn) {
+			of_aliases = NULL;
+			of_aliases_forget_all();
+			of_node_put(rd->dn);
+		}
+		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);
+#endif /* CONFIG_OF_DYNAMIC */
+
 /**
  * of_alias_scan - Scan all properties of the 'aliases' node
  * @dt_alloc:	An allocator that provides a virtual address to memory
@@ -1961,42 +2114,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 (end > start && isdigit(*(end - 1)))
-			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);
 }
 
 /**
@@ -2014,7 +2133,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;
@@ -2024,7 +2143,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;
 }
@@ -2042,7 +2161,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;
@@ -2050,7 +2169,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 be4e1584e0af..5ca249fca02f 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -238,7 +238,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,
@@ -246,7 +246,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 9bba999f0bf8..731b606ae3e3 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;
 
@@ -223,6 +230,13 @@ static inline bool of_alias_value_ok(const struct property *pp)
 	       strnlen(pp->value, pp->length) < pp->length;
 }
 
+/* the /aliases node: root child with the exact name "aliases" */
+static inline bool of_node_is_aliases(const struct device_node *np)
+{
+	return of_node_is_root(np->parent) &&
+	       !strcmp(kbasename(np->full_name), "aliases");
+}
+
 #if IS_ENABLED(CONFIG_KUNIT)
 int __of_address_resource_bounds(struct resource *r, u64 start, u64 size);
 #endif

-- 
2.54.0

[PATCH v7 03/10] of/overlay: look up absolute target-paths absolutely

[Abdurrahman Hussain]

find_target() with a non-NULL target base concatenates the base path
and the fragment's target-path via "%pOF%s": target-path="/foo"
resolves to "<base>/foo" and can never reach the DT root. An overlay
applied with a base cannot both extend the base subtree and add
/aliases entries, which is what a PCI device declaring its
peripherals under dev_of_node() needs for alias-based bus numbering.

Treat any non-empty target-path as absolute. An empty target-path
still means the base itself, the only form used by the one in-tree
caller passing a base (drivers/misc/lan966x_pci.c).

Spell the contract out in the kernel-doc for @base/@target_base and
in find_target()'s strategy comment.

Assisted-by: Claude:claude-fable-5 [Claude Code]
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/of/overlay.c | 38 ++++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 08d5351746be..74aea704835a 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -688,12 +688,15 @@ static int build_changeset(struct overlay_changeset *ovcs)
  *
  * 1) "target" property containing the phandle of the target
  * 2) "target-path" property containing the path of the target
+ *
+ * With a non-NULL @target_base, an empty "target-path" means
+ * @target_base itself; any non-empty "target-path" is resolved
+ * absolutely from the live-tree root.
  */
 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 +712,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;
 	}
 
@@ -737,7 +731,9 @@ static struct device_node *find_target(const struct device_node *info_node,
 /**
  * init_overlay_changeset() - initialize overlay changeset from overlay tree
  * @ovcs:		Overlay changeset to build
- * @target_base:	Point to the target node to apply overlay
+ * @target_base:	Target for fragments with an empty "target-path";
+ *			fragments with a non-empty "target-path" resolve
+ *			absolutely and ignore @target_base
  *
  * Initialize @ovcs.  Populate @ovcs->fragments with node information from
  * the top level of @overlay_root.  The relevant top level nodes are the
@@ -982,7 +978,9 @@ static int of_overlay_apply(struct overlay_changeset *ovcs,
  * @overlay_fdt:	pointer to overlay FDT
  * @overlay_fdt_size:	number of bytes in @overlay_fdt
  * @ret_ovcs_id:	pointer for returning created changeset id
- * @base:		pointer for the target node to apply overlay
+ * @base:		target for fragments with an empty "target-path";
+ *			fragments with a non-empty "target-path" resolve
+ *			absolutely and ignore @base
  *
  * Creates and applies an overlay changeset.
  *

-- 
2.54.0

[PATCH v7 04/10] of/overlay: put property on deadprops only after changeset add succeeds

[Abdurrahman Hussain]

add_changeset_property() links a new property of a not-yet-live
target node into the node's deadprops list before handing it to
of_changeset_add_property(). If that fails, the error path frees the
property but leaves the freed pointer linked in deadprops, and
of_node_release() frees it a second time when the aborted overlay's
node is released.

Record the changeset entry first and link the property into deadprops
only on success. of_changeset_add_property() never looks at the
node's property lists, so the order of the two steps is otherwise
immaterial, and the error path frees a property that nothing
references.

Fixes: f96278810150 ("of: overlay: set node fields from properties when add new overlay node")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-fable-5 [Claude Code]
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/of/overlay.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 74aea704835a..284c9bc6c9cf 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -358,12 +358,13 @@ static int add_changeset_property(struct overlay_changeset *ovcs,
 		return -ENOMEM;
 
 	if (!prop) {
-		if (!target->in_livetree) {
+		ret = of_changeset_add_property(&ovcs->cset, target->np,
+						new_prop);
+		/* the detached node owns the property until the apply */
+		if (!ret && !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 {
 		ret = of_changeset_update_property(&ovcs->cset, target->np,
 						   new_prop);

-- 
2.54.0

[PATCH v7 05/10] of/overlay: only treat a positive changeset id as registered

[Abdurrahman Hussain]

of_overlay_fdt_apply() stores the idr_alloc() return value in
ovcs->id before checking it. On failure the stored id is negative,
free_overlay_changeset()'s "if (ovcs->id)" check passes, idr_remove()
is called with a negative id and list_del() runs on ovcs->ovcs_list,
which is not initialized until after the id allocation. An allocation
failure at that point dereferences NULL.

Make free_overlay_changeset() treat only a strict-positive id as
registered. The rest of the function already copes with a
partially-initialized ovcs, so the error path stays a plain
goto err_free_ovcs.

Fixes: 61b4de4e0b38 ("of: overlay: minor restructuring")
Cc: stable@vger.kernel.org
Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
Assisted-by: Claude:claude-fable-5 [Claude Code]
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/of/overlay.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 284c9bc6c9cf..9b9f198a1d70 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -860,7 +860,8 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
 	if (ovcs->cset.entries.next)
 		of_changeset_destroy(&ovcs->cset);
 
-	if (ovcs->id) {
+	/* a failed idr_alloc() leaves its negative error in ovcs->id */
+	if (ovcs->id > 0) {
 		idr_remove(&ovcs_idr, ovcs->id);
 		list_del(&ovcs->ovcs_list);
 		ovcs->id = 0;

-- 
2.54.0

[PATCH v7 06/10] of/overlay: don't leak fragment references when changeset init fails

[Abdurrahman Hussain]

init_overlay_changeset() stores the number of initialized fragments
in ovcs->count only on full success. When find_target() or the
__symbols__ lookup fails partway through, the target and overlay
references taken for the fragments initialized so far are never
dropped: free_overlay_changeset() bounds its cleanup loop by
ovcs->count, which is still 0.

Store the running count on the error path so free_overlay_changeset()
puts whatever was set up. The store is guarded by ovcs->fragments
because on the allocation-failure path cnt still holds the counting
pass total while there is no fragments array; everywhere else cnt
only counts fully-initialized fragments.

Reported-by: Sashiko AI <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/20260805204009.CF3891F000E9@smtp.kernel.org
Fixes: 61b4de4e0b38 ("of: overlay: minor restructuring")
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/of/overlay.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 9b9f198a1d70..51241f16e87b 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -850,6 +850,10 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs,
 err_out:
 	pr_err("%s() failed, ret = %d\n", __func__, ret);
 
+	/* let free_overlay_changeset() put the fragments set up so far */
+	if (ovcs->fragments)
+		ovcs->count = cnt;
+
 	return ret;
 }
 

-- 
2.54.0

[PATCH v7 07/10] of/overlay: don't create "//" paths for fragments targeting the root

[Abdurrahman Hussain]

dup_and_fixup_symbol_prop() rewrites a symbol value by replacing its
"/fragment/__overlay__" prefix with the fragment's target path. When
the fragment targets the root node the target path is "/" and the
result starts with "//", which __of_find_node_by_full_path() cannot
resolve: symbols pointing into such fragments silently stop
resolving.

Drop the target path when it is the root and the tail is non-empty. A
value naming the fragment root itself (empty tail) keeps the "/".

Fixes: d1651b03c2df ("of: overlay: add overlay symbols to live device tree")
Assisted-by: Claude:claude-fable-5 [Claude Code]
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/of/overlay.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 51241f16e87b..6f2d7872028a 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -256,6 +256,9 @@ static struct property *dup_and_fixup_symbol_prop(
 	if (!target_path)
 		return NULL;
 	target_path_len = strlen(target_path);
+	/* a root target renders as "/"; drop it to avoid "//" results */
+	if (target_path_len == 1 && target_path[0] == '/' && path_tail_len)
+		target_path_len = 0;
 
 	new_prop = kzalloc_obj(*new_prop);
 	if (!new_prop)

-- 
2.54.0

[PATCH v7 08/10] of/overlay: return ERR_PTR from dup_and_fixup_symbol_prop()

[Abdurrahman Hussain]

dup_and_fixup_symbol_prop() returns NULL for malformed values, for
values that are not paths into one of the overlay's fragments, and
for allocation failures. The caller reports all of them as -ENOMEM,
so a structural problem in /__symbols__ is diagnosed as memory
exhaustion.

A later patch reuses the helper for /aliases values and must handle
the three cases differently: copy verbatim, warn, or fail the apply.
Return ERR_PTR(-EINVAL), ERR_PTR(-ENODEV) and ERR_PTR(-ENOMEM)
respectively and propagate the errno in the /__symbols__ caller.

Also verify that the value descends through the matched fragment's
__overlay__ node before cutting the prefix. Only the first path
component was resolved, so an absolute live-tree value sharing its
first component with a fragment name was sliced at the prefix length
and rewritten to garbage. A prefix mismatch returns -ENODEV.

Assisted-by: Claude:claude-fable-5 [Claude Code]
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/of/overlay.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 6f2d7872028a..bc0c879dd807 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -206,6 +206,10 @@ static void overlay_fw_devlink_refresh(struct overlay_changeset *ovcs)
  * The duplicated property value will be modified by replacing the
  * "/fragment_name/__overlay/" portion of the value  with the target
  * path from the fragment node.
+ *
+ * Return: the fixed-up property, or ERR_PTR: -EINVAL if @prop's value
+ * is not a valid non-empty C string, -ENODEV if it is not a path into
+ * one of @ovcs's fragments, -ENOMEM on allocation failure.
  */
 static struct property *dup_and_fixup_symbol_prop(
 		struct overlay_changeset *ovcs, const struct property *prop)
@@ -217,6 +221,8 @@ static struct property *dup_and_fixup_symbol_prop(
 	const char *path;
 	const char *path_tail;
 	const char *target_path;
+	char *overlay_name;
+	bool mismatch;
 	int k;
 	int overlay_name_len;
 	int path_len;
@@ -224,14 +230,14 @@ static struct property *dup_and_fixup_symbol_prop(
 	int target_path_len;
 
 	if (!prop->value)
-		return NULL;
+		return ERR_PTR(-EINVAL);
 	if (strnlen(prop->value, prop->length) >= prop->length)
-		return NULL;
+		return ERR_PTR(-EINVAL);
 	path = prop->value;
 	path_len = strlen(path);
 
 	if (path_len < 1)
-		return NULL;
+		return ERR_PTR(-EINVAL);
 	fragment_node = __of_find_node_by_path(ovcs->overlay_root, path + 1);
 	overlay_node = __of_find_node_by_path(fragment_node, "__overlay__/");
 	of_node_put(fragment_node);
@@ -243,18 +249,27 @@ static struct property *dup_and_fixup_symbol_prop(
 			break;
 	}
 	if (k >= ovcs->count)
-		return NULL;
+		return ERR_PTR(-ENODEV);
+
+	overlay_name = kasprintf(GFP_KERNEL, "%pOF", fragment->overlay);
+	if (!overlay_name)
+		return ERR_PTR(-ENOMEM);
+	overlay_name_len = strlen(overlay_name);
 
-	overlay_name_len = snprintf(NULL, 0, "%pOF", fragment->overlay);
+	/* @path must descend through this fragment's __overlay__ node */
+	mismatch = overlay_name_len > path_len ||
+		   strncmp(path, overlay_name, overlay_name_len) != 0 ||
+		   (path[overlay_name_len] != '/' && path[overlay_name_len]);
+	kfree(overlay_name);
+	if (mismatch)
+		return ERR_PTR(-ENODEV);
 
-	if (overlay_name_len > path_len)
-		return NULL;
 	path_tail = path + overlay_name_len;
 	path_tail_len = strlen(path_tail);
 
 	target_path = kasprintf(GFP_KERNEL, "%pOF", fragment->target);
 	if (!target_path)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 	target_path_len = strlen(target_path);
 	/* a root target renders as "/"; drop it to avoid "//" results */
 	if (target_path_len == 1 && target_path[0] == '/' && path_tail_len)
@@ -284,7 +299,7 @@ static struct property *dup_and_fixup_symbol_prop(
 err_free_target_path:
 	kfree(target_path);
 
-	return NULL;
+	return ERR_PTR(-ENOMEM);
 }
 
 /**
@@ -353,6 +368,8 @@ static int add_changeset_property(struct overlay_changeset *ovcs,
 		if (prop)
 			return -EINVAL;
 		new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop);
+		if (IS_ERR(new_prop))
+			return PTR_ERR(new_prop);
 	} else {
 		new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL);
 	}

-- 
2.54.0

[PATCH v7 09/10] of/overlay: rewrite /aliases path values to live-tree paths

[Abdurrahman Hussain]

An /aliases property added by an overlay references labeled nodes as
"&label", which dtc renders as an overlay-internal path such as
"/fragment@1/__overlay__/i2c@40000". The value is copied verbatim
into the live tree, of_find_node_by_path() cannot resolve it, and
of_alias_get_id() keeps returning -ENODEV.

/__symbols__ values follow the same convention and are already
rewritten by dup_and_fixup_symbol_prop(). Use it for properties of
the /aliases node too:

  - a value that resolves inside one of the overlay's fragments is
    stored rewritten to the live-tree path
  - -ENODEV (legacy string aliases, absolute live-tree paths) is
    copied verbatim, as before this series
  - -EINVAL (not a NUL-terminated string) is copied verbatim with a
    warning; consumers validate before dereferencing, and an overlay
    that applied before this series keeps applying
  - only -ENOMEM fails the apply

Pseudo-properties are exempt: the is_pseudo_property() check at the
top of add_changeset_property() only covers live-tree targets, and a
phandle of a newly created /aliases node is a cell, not a string.

Assisted-by: Claude:claude-fable-5 [Claude Code]
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/of/overlay.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index bc0c879dd807..2af9ffb08691 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -370,6 +370,19 @@ static int add_changeset_property(struct overlay_changeset *ovcs,
 		new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop);
 		if (IS_ERR(new_prop))
 			return PTR_ERR(new_prop);
+	} else if (!is_pseudo_property(overlay_prop->name) &&
+		   of_node_is_aliases(target->np)) {
+		/* rewrite overlay-internal alias values to live-tree paths */
+		new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop);
+		if (new_prop == ERR_PTR(-ENOMEM))
+			return -ENOMEM;
+		if (IS_ERR(new_prop)) {
+			if (new_prop == ERR_PTR(-EINVAL))
+				pr_warn("%pOF/%s is not a valid string; alias will be inert\n",
+					target->np, overlay_prop->name);
+			/* not overlay-internal: copy verbatim, consumers validate */
+			new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL);
+		}
 	} else {
 		new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL);
 	}

-- 
2.54.0

[PATCH v7 10/10] of: unittest: cover /aliases updates from overlay apply/revert

[Abdurrahman Hussain]

Add overlay_alias.dtso and of_unittest_overlay_alias().

The overlay has two fragments: target-path="" grafts a labeled node
under the run-time target base, and target-path="/aliases" adds
`testcase-alias99 = &<label>` at the DT root. The runner applies it
with a non-NULL base via of_overlay_fdt_apply() (overlay_data_apply()
always passes NULL), asserts of_alias_get_id() on the grafted node
returns 99, reverts, and asserts of_alias_get_highest_id() reports
the stem gone. A precondition check runs before the apply.

This exercises the reconfig notifier, the absolute target-path
lookup, and the /aliases value rewrite together; any one missing
turns the get_id assertion into -ENODEV.

The grafted-node reference is dropped before of_overlay_remove():
overlay-created nodes must be back at refcount 1 when the changeset
is destroyed. The post-remove assertion uses of_alias_get_highest_id()
because it is keyed by stem and needs no node reference. The apply is
wrapped in EXPECT_BEGIN/END for the "memory leak will occur" warning
printed for properties added to nodes the overlay didn't create,
like the other overlay tests.

The overlay is not added to the fdtoverlay static build test:
fdtoverlay has no target-base concept, so the empty target-path
cannot be resolved statically.

Assisted-by: Claude:claude-fable-5 [Claude Code]
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                       | 73 +++++++++++++++++++++++++++++
 3 files changed, 103 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..2f5e4056efae 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -3475,6 +3475,77 @@ 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;
+	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;
+	}
+
+	id = of_alias_get_highest_id("testcase-alias");
+	if (id != -ENODEV) {
+		unittest(0, "unexpected testcase-alias%d before overlay apply\n",
+			 id);
+		goto out_put_base;
+	}
+
+	EXPECT_BEGIN(KERN_INFO,
+		     "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /aliases/testcase-alias99");
+
+	ret = of_overlay_fdt_apply(__dtbo_overlay_alias_begin, size,
+				   &ovcs_id, base);
+
+	EXPECT_END(KERN_INFO,
+		   "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /aliases/testcase-alias99");
+
+	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;
+	}
+
+	target = of_find_node_by_path(target_path);
+	if (!target) {
+		unittest(0, "could not find grafted target %s\n", target_path);
+	} else {
+		id = of_alias_get_id(target, "testcase-alias");
+		unittest(id == 99,
+			 "of_alias_get_id() = %d after overlay apply, expected 99\n",
+			 id);
+		/* changeset destroy expects overlay nodes back at refcount 1 */
+		of_node_put(target);
+	}
+
+	ret = of_overlay_remove(&ovcs_id);
+	if (ret) {
+		unittest(0, "overlay_alias remove failed, ret = %d\n", ret);
+		goto out_put_base;
+	}
+
+	id = of_alias_get_highest_id("testcase-alias");
+	unittest(id == -ENODEV,
+		 "of_alias_get_highest_id() = %d after overlay remove, expected -ENODEV\n",
+		 id);
+
+out_put_base:
+	of_node_put(base);
+}
+
 static void __init of_unittest_overlay_notify(void)
 {
 	int ovcs_id;
@@ -3649,6 +3720,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

Re: [PATCH v7 02/10] of: update /aliases lookup on reconfig notifications

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Lock-free access to the global `of_aliases` pointer in `of_find_node_opts_by_path()` introduces a TOCTOU Use-After-Free race with dynamic overlay detachments.
--

commit e8d4ce869294e7247761b17acbbfb6ac98d74f68
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

of: update /aliases lookup on reconfig notifications

This commit registers a reconfig notifier to mirror /aliases property changes
into aliases_lookup. The notifier chain covers changesets and overlays, matching
/aliases by name and root parent, and updates runtime-allocated entries.

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]

>     The reference makes the lock-free
>     of_node_get(of_aliases) in of_find_node_opts_by_path() safe: a reader
>     that observes a non-NULL pointer observes a live node, and on DETACH
>     the changeset holds its own references across the notifier so the put
>     here cannot be the final one while readers still walk the node.

[Severity: High]
Is this description accurate?

If a thread calls of_find_node_opts_by_path() and is preempted immediately after
reading the of_aliases pointer but before calling of_node_get(), it could be
delayed arbitrarily.

If an overlay detachment completes concurrently, it drops the notifier's
reference and fully destroys the overlay changeset. When the preempted thread
resumes, wouldn't it call of_node_get() on freed memory?

> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index f7aa14d90e501..23d9bb073d8b5 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 || !of_node_is_aliases(rd->dn))
> +		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:
> +		if (of_aliases == rd->dn) {
> +			of_aliases = NULL;
> +			of_aliases_forget_all();
> +			of_node_put(rd->dn);

[Severity: High]
Can this introduce a Use-After-Free race with dynamic overlay detachments?

This overlay detachment clears the global pointer and drops the reference.
As discussed above, a lock-free reader could be preempted in
of_find_node_opts_by_path() right before acquiring the reference:

    aliases = of_node_get(of_aliases);

If the reader is delayed until after the overlay changeset is destroyed,
the node memory can be freed. Will the preempted thread then dereference
freed memory when it finally calls of_node_get()?

> +		}
> +		break;
> +	case OF_RECONFIG_ADD_PROPERTY:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-nh-of-alias-overlay-v7-0-02754604805a@nexthop.ai?part=2

Re: [PATCH v7 05/10] of/overlay: only treat a positive changeset id as registered

[sashiko-bot]

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

Pre-existing issues:
- [High] In `of_overlay_remove_all()`, passing `&ovcs->id` to `of_overlay_remove()` causes a deterministic use-after-free and leaves freed memory in the global `ovcs_list` and `ovcs_idr`.
- [High] Lockless iteration of `ovcs_list` in `of_overlay_remove_all()` allows concurrent modifications, leading to a Use-After-Free.
--

commit 5e44e19c9c02429165243b3c23378b30ba40fdd3
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

of/overlay: only treat a positive changeset id as registered

of_overlay_fdt_apply() stores the idr_alloc() return value in
ovcs->id before checking it. On failure the stored id is negative,
free_overlay_changeset()'s "if (ovcs->id)" check passes, idr_remove()
is called with a negative id and list_del() runs on ovcs->ovcs_list,
which is not initialized until after the id allocation. An allocation
failure at that point dereferences NULL.

> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 08d5351746be6..9b9f198a1d70a 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
[ ... ]
> @@ -860,7 +860,8 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
>  	if (ovcs->cset.entries.next)
>  		of_changeset_destroy(&ovcs->cset);
>  
> -	if (ovcs->id) {
> +	/* a failed idr_alloc() leaves its negative error in ovcs->id */
> +	if (ovcs->id > 0) {

[Severity: High]
This isn't a bug introduced by this patch, but does this check cause a 
use-after-free and list corruption when called from of_overlay_remove_all()?

In of_overlay_remove_all(), we iterate over the ovcs_list and pass the 
address of the internal id field:

drivers/of/overlay.c:of_overlay_remove_all() {
    list_for_each_entry_safe_reverse(ovcs, ovcs_n, &ovcs_list, ovcs_list) {
        ret = of_overlay_remove(&ovcs->id);
...
}

Inside of_overlay_remove(), it zeros out this ID directly through the pointer 
before freeing the changeset:

drivers/of/overlay.c:of_overlay_remove() {
...
    *ovcs_id = 0;
...
    free_overlay_changeset(ovcs);
...
}

When free_overlay_changeset() executes, ovcs->id is already 0, so the 
if (ovcs->id > 0) condition will evaluate to false. This skips both 
idr_remove() and list_del(&ovcs->ovcs_list) before the memory is freed, 
leaving stale pointers in the active global list and IDR.


[Severity: High]
This is also a pre-existing issue, but could the lockless iteration in 
of_overlay_remove_all() lead to a use-after-free?

The iteration of ovcs_list in of_overlay_remove_all() allows concurrent 
modifications:

drivers/of/overlay.c:of_overlay_remove_all() {
    list_for_each_entry_safe_reverse(ovcs, ovcs_n, &ovcs_list, ovcs_list) {
...
}

This iterates the globally shared ovcs_list without holding of_mutex. If 
another thread concurrently calls of_overlay_remove() on a different overlay, 
it will acquire of_mutex and remove or free that overlay.

If the freed overlay was concurrently cached as ovcs_n (the 'next' element in 
the safe iteration), the loop in of_overlay_remove_all() will dereference 
the freed memory when advancing to the next iteration.

>  		idr_remove(&ovcs_idr, ovcs->id);
>  		list_del(&ovcs->ovcs_list);
>  		ovcs->id = 0;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-nh-of-alias-overlay-v7-0-02754604805a@nexthop.ai?part=5

Re: [PATCH v7 01/10] of: hold a reference on of_aliases during alias path resolution

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Lockless load of the global `of_aliases` pointer followed by `of_node_get()` creates a Time-Of-Check to Time-Of-Use (TOCTOU) race condition that can lead to a Use-After-Free.
--

commit 6d7d618928c35a7237faefa2eebe21725f461ed9
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

of: hold a reference on of_aliases during alias path resolution
    
This commit updates of_find_node_opts_by_path() to hold a reference on the
of_aliases node during alias path resolution. It also adds validation for
alias values, checking that they are non-empty, NUL-terminated absolute paths.

> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 378703dbc11f3..f7aa14d90e501 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
[ ... ]
> @@ -1002,16 +1004,21 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt
>  			p = separator;
>  		len = p - path;
>  
> -		/* of_aliases must not be NULL */
> -		if (!of_aliases)
> +		aliases = of_node_get(of_aliases);

[Severity: High]
Does this lockless load of the global of_aliases pointer followed by
of_node_get() create a Time-Of-Check to Time-Of-Use (TOCTOU) race condition?

If a reader thread evaluating of_aliases in of_find_node_opts_by_path() is
preempted immediately after loading the pointer into a register, but before
executing kobject_get() inside of_node_get(), a concurrent writer thread
could completely remove the overlay containing the aliases node.

The writer thread would drop all references and immediately free the node.
When the reader thread resumes, it would pass the freed pointer to
of_node_get(), which would dereference it to increment the refcount,
resulting in a use-after-free.

> +		if (!aliases)
>  			return NULL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-nh-of-alias-overlay-v7-0-02754604805a@nexthop.ai?part=1

Re: [PATCH v7 05/10] of/overlay: only treat a positive changeset id as registered

[Abdurrahman Hussain]

On Mon Aug 31, 2026 at 6:45 PM PDT, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] In `of_overlay_remove_all()`, passing `&ovcs->id` to `of_overlay_remove()` causes a deterministic use-after-free and leaves freed memory in the global `ovcs_list` and `ovcs_idr`.
> - [High] Lockless iteration of `ovcs_list` in `of_overlay_remove_all()` allows concurrent modifications, leading to a Use-After-Free.
> --
>
> commit 5e44e19c9c02429165243b3c23378b30ba40fdd3
> Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>
>
> of/overlay: only treat a positive changeset id as registered
>
> of_overlay_fdt_apply() stores the idr_alloc() return value in
> ovcs->id before checking it. On failure the stored id is negative,
> free_overlay_changeset()'s "if (ovcs->id)" check passes, idr_remove()
> is called with a negative id and list_del() runs on ovcs->ovcs_list,
> which is not initialized until after the id allocation. An allocation
> failure at that point dereferences NULL.
>
>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>> index 08d5351746be6..9b9f198a1d70a 100644
>> --- a/drivers/of/overlay.c
>> +++ b/drivers/of/overlay.c
> [ ... ]
>> @@ -860,7 +860,8 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
>>  	if (ovcs->cset.entries.next)
>>  		of_changeset_destroy(&ovcs->cset);
>>  
>> -	if (ovcs->id) {
>> +	/* a failed idr_alloc() leaves its negative error in ovcs->id */
>> +	if (ovcs->id > 0) {
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does this check cause a 
> use-after-free and list corruption when called from of_overlay_remove_all()?
>
> In of_overlay_remove_all(), we iterate over the ovcs_list and pass the 
> address of the internal id field:
>
> drivers/of/overlay.c:of_overlay_remove_all() {
>     list_for_each_entry_safe_reverse(ovcs, ovcs_n, &ovcs_list, ovcs_list) {
>         ret = of_overlay_remove(&ovcs->id);
> ...
> }
>
> Inside of_overlay_remove(), it zeros out this ID directly through the pointer 
> before freeing the changeset:
>
> drivers/of/overlay.c:of_overlay_remove() {
> ...
>     *ovcs_id = 0;
> ...
>     free_overlay_changeset(ovcs);
> ...
> }
>
> When free_overlay_changeset() executes, ovcs->id is already 0, so the 
> if (ovcs->id > 0) condition will evaluate to false. This skips both 
> idr_remove() and list_del(&ovcs->ovcs_list) before the memory is freed, 
> leaving stale pointers in the active global list and IDR.
>

Confirmed. of_overlay_remove_all() is the one caller that passes
&ovcs->id, so the "*ovcs_id = 0" in of_overlay_remove() zeroes
ovcs->id itself before free_overlay_changeset() runs. The
"if (ovcs->id > 0)" guard (previously "if (ovcs->id)") then skips
idr_remove() and list_del(), and the ovcs is kfree()d while still
linked in ovcs_list and registered in ovcs_idr. The next ovcs_list or
ovcs_idr access after that walks freed memory.

Pre-existing — the ordering dates to 24789c5ce5a3 ("of: overlay:
detect cases where device tree may become corrupt") — and latent, since
of_overlay_remove_all() currently has no in-tree callers. Still worth
fixing for module users. The minimal fix is to stop aliasing the
changeset's own field:

      list_for_each_entry_safe_reverse(ovcs, ovcs_n, &ovcs_list, ovcs_list) {
-             ret = of_overlay_remove(&ovcs->id);
+             int id = ovcs->id;
+
+             ret = of_overlay_remove(&id);

Rob, want this as one more patch in this series (it sits right next to
patch 5), or as a separate fix?

>
> [Severity: High]
> This is also a pre-existing issue, but could the lockless iteration in 
> of_overlay_remove_all() lead to a use-after-free?
>
> The iteration of ovcs_list in of_overlay_remove_all() allows concurrent 
> modifications:
>
> drivers/of/overlay.c:of_overlay_remove_all() {
>     list_for_each_entry_safe_reverse(ovcs, ovcs_n, &ovcs_list, ovcs_list) {
> ...
> }
>
> This iterates the globally shared ovcs_list without holding of_mutex. If 
> another thread concurrently calls of_overlay_remove() on a different overlay, 
> it will acquire of_mutex and remove or free that overlay.
>
> If the freed overlay was concurrently cached as ovcs_n (the 'next' element in 
> the safe iteration), the loop in of_overlay_remove_all() will dereference 
> the freed memory when advancing to the next iteration.
>
>>  		idr_remove(&ovcs_idr, ovcs->id);
>>  		list_del(&ovcs->ovcs_list);
>>  		ovcs->id = 0;
>>  	}

Known — you raised the same walk on v6, and it was deliberately left
out of this series' scope. It also can't be fixed locally: wrapping
the loop in of_mutex deadlocks against of_overlay_remove() taking
of_mutex itself, so it needs a __of_overlay_remove() split. That is
follow-up material, and equally unreachable in-tree today for the same
no-callers reason.

Thanks,
Abdurrahman

Re: [PATCH v7 01/10] of: hold a reference on of_aliases during alias path resolution

[Abdurrahman Hussain]

On Mon Aug 31, 2026 at 6:55 PM PDT, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Lockless load of the global `of_aliases` pointer followed by `of_node_get()` creates a Time-Of-Check to Time-Of-Use (TOCTOU) race condition that can lead to a Use-After-Free.
> --
>
> commit 6d7d618928c35a7237faefa2eebe21725f461ed9
> Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>
>
> of: hold a reference on of_aliases during alias path resolution
>     
> This commit updates of_find_node_opts_by_path() to hold a reference on the
> of_aliases node during alias path resolution. It also adds validation for
> alias values, checking that they are non-empty, NUL-terminated absolute paths.
>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index 378703dbc11f3..f7aa14d90e501 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
> [ ... ]
>> @@ -1002,16 +1004,21 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt
>>  			p = separator;
>>  		len = p - path;
>>  
>> -		/* of_aliases must not be NULL */
>> -		if (!of_aliases)
>> +		aliases = of_node_get(of_aliases);
>
> [Severity: High]
> Does this lockless load of the global of_aliases pointer followed by
> of_node_get() create a Time-Of-Check to Time-Of-Use (TOCTOU) race condition?
>
> If a reader thread evaluating of_aliases in of_find_node_opts_by_path() is
> preempted immediately after loading the pointer into a register, but before
> executing kobject_get() inside of_node_get(), a concurrent writer thread
> could completely remove the overlay containing the aliases node.
>
> The writer thread would drop all references and immediately free the node.
> When the reader thread resumes, it would pass the freed pointer to
> of_node_get(), which would dereference it to increment the refcount,
> resulting in a use-after-free.
>
>> +		if (!aliases)
>>  			return NULL;

In the strictest sense that window exists, but consider what it takes:
the reader must be preempted on the single instruction between the
pointer load and the refcount increment, and stay off-CPU while an
entire overlay teardown — changeset revert, notifiers, and
of_changeset_destroy(), all under of_mutex — starts and finishes. And
it only opens at all when /aliases itself is dynamically deleted,
i.e. the base DT booted without /aliases and the one overlay that
created it is reverted concurrently with an alias lookup. of_aliases
always holds a reference on the node it points to, the DETACH path
clears the pointer before dropping that reference, and the changeset
pins the node from the notifier until of_changeset_destroy() — so
there is no point at which a reader can load a pointer to an
already-freed node.

Closing the residual gap means serializing the reader against the
final of_node_put(). v6 did exactly that (devtree_lock around the
load + get) and Rob asked for the plain of_node_get() instead:

  https://lore.kernel.org/r/<rob-v6-reply-msgid>

The remaining options were already explored in this series' history:
keeping the DETACH reference (v3) trips __of_changeset_entry_destroy()'s
refcount check and leaks the node every apply/revert cycle, and putting
node lifetimes under RCU is a tree-wide change far beyond this series.
I'm keeping Rob's requested form.

Thanks,
Abdurrahman