From: Rob Herring <robh@kernel.org>
To: Abdurrahman Hussain <abdurrahman@nexthop.ai>
Cc: Saravana Kannan <saravanak@kernel.org>,
Frank Rowand <frowand.list@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Shawn Guo <shawnguo@kernel.org>,
Grant Likely <grant.likely@secretlab.ca>,
Grant Likely <grant.likely@linaro.org>,
Pantelis Antoniou <pantelis.antoniou@konsulko.com>,
David Daney <david.daney@cavium.com>,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org, Geert Uytterhoeven <geert@linux-m68k.org>,
Geert Uytterhoeven <geert+renesas@glider.be>,
Sashiko AI <sashiko-bot@kernel.org>
Subject: Re: [PATCH v7 00/10] of: teach overlay code to keep /aliases in sync
Date: Thu, 17 Sep 2026 15:39:38 -0500 [thread overview]
Message-ID: <20260917203938.GA3480871-robh@kernel.org> (raw)
In-Reply-To: <20260831-nh-of-alias-overlay-v7-0-02754604805a@nexthop.ai>
On Mon, Aug 31, 2026 at 06:29:18PM -0700, Abdurrahman Hussain 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.
>
> 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.
There was another posting in 2024 of Geert's patches and AFAICT my
comment there[1] still applies. To repeat, what happens if the alias
number already got used because the subsystems can pick any of the
numbers without an alias. The only way I see to solve that is make
possible alias numbers and dynamic numbers non-overlapping or only allow
alias names that are not present in the base DT to be used/honored in
overlays.
> 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.
Fixes should come first in a series. Then I can apply them if ready even
if the feature patches are not ready.
Rob
[1] https://lore.kernel.org/all/CAL_Jsq+72Q6LyOj1va_qcyCVkSRwqGNvBFfB9NNOgYXasAFYJQ@mail.gmail.com/
next prev parent reply other threads:[~2026-09-17 20:39 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-01 1:29 [PATCH v7 00/10] of: teach overlay code to keep /aliases in sync Abdurrahman Hussain
2026-09-01 1:29 ` [PATCH v7 01/10] of: hold a reference on of_aliases during alias path resolution Abdurrahman Hussain
2026-09-01 1:55 ` sashiko-bot
2026-09-03 23:17 ` Abdurrahman Hussain
2026-09-01 1:29 ` [PATCH v7 02/10] of: update /aliases lookup on reconfig notifications Abdurrahman Hussain
2026-09-01 1:45 ` sashiko-bot
2026-09-01 1:29 ` [PATCH v7 03/10] of/overlay: look up absolute target-paths absolutely Abdurrahman Hussain
2026-09-01 1:29 ` [PATCH v7 04/10] of/overlay: put property on deadprops only after changeset add succeeds Abdurrahman Hussain
2026-09-01 1:29 ` [PATCH v7 05/10] of/overlay: only treat a positive changeset id as registered Abdurrahman Hussain
2026-09-01 1:45 ` sashiko-bot
2026-09-03 23:11 ` Abdurrahman Hussain
2026-09-01 1:29 ` [PATCH v7 06/10] of/overlay: don't leak fragment references when changeset init fails Abdurrahman Hussain
2026-09-01 1:29 ` [PATCH v7 07/10] of/overlay: don't create "//" paths for fragments targeting the root Abdurrahman Hussain
2026-09-01 1:29 ` [PATCH v7 08/10] of/overlay: return ERR_PTR from dup_and_fixup_symbol_prop() Abdurrahman Hussain
2026-09-01 1:29 ` [PATCH v7 09/10] of/overlay: rewrite /aliases path values to live-tree paths Abdurrahman Hussain
2026-09-01 1:29 ` [PATCH v7 10/10] of: unittest: cover /aliases updates from overlay apply/revert Abdurrahman Hussain
2026-09-17 20:39 ` Rob Herring [this message]
2026-09-18 2:21 ` [PATCH v7 00/10] of: teach overlay code to keep /aliases in sync Abdurrahman Hussain
2026-09-17 21:41 ` Rob Herring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260917203938.GA3480871-robh@kernel.org \
--to=robh@kernel.org \
--cc=abdurrahman@nexthop.ai \
--cc=davem@davemloft.net \
--cc=david.daney@cavium.com \
--cc=devicetree@vger.kernel.org \
--cc=frowand.list@gmail.com \
--cc=geert+renesas@glider.be \
--cc=geert@linux-m68k.org \
--cc=grant.likely@linaro.org \
--cc=grant.likely@secretlab.ca \
--cc=linux-kernel@vger.kernel.org \
--cc=pantelis.antoniou@konsulko.com \
--cc=saravanak@kernel.org \
--cc=sashiko-bot@kernel.org \
--cc=shawnguo@kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.