Re: [PATCH v4 2/6] of: incrementally update /aliases lookup on reconfig notifications

[Krzysztof Kozlowski <krzk@kernel.org>]

On Tue, Jul 21, 2026 at 06:01:47PM -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.
> 
> 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 (exact name "aliases",
> parent == root, via the shared of_node_is_aliases()) 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 name compare is exact rather than of_node_name_eq():
> the latter ignores unit addresses and would also match a root node
> named "aliases@1", which neither path lookup nor the DT spec treats
> as the aliases node. ATTACH publishes the adopted node
> in of_aliases with a reference held; DETACH clears the pointer and
> drops that reference again. Both pointer updates happen under
> devtree_lock, pairing with the locked reader in
> of_find_node_opts_by_path() from the previous patch — a reader either
> observes NULL or takes its own reference before the notifier's put
> can be the last one. Dropping the reference at DETACH also keeps the
> node at refcount 1 by the time an overlay changeset that created
> /aliases is destroyed, which __of_changeset_entry_destroy() insists
> on before it lets the node be freed.
> 
> 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 — but only
> when the detached node is the tracked of_aliases. __of_attach_node()
> has no duplicate-name check, so a stray second root node named
> "aliases" can exist; detaching it must not wipe entries backed by the
> real node. 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. The per-entry teardown is
> factored into __of_alias_del(), shared by the single-name destroy and
> the detach-time sweep.
> 
> One ordering caveat is inherent to the notification architecture:
> within a single changeset, a device created by an earlier ATTACH
> entry can be probed by of_platform_notify() before a later /aliases
> ADD_PROPERTY entry reaches this notifier. Overlays that declare an
> alias for a node they also create should order the /aliases fragment
> first if the target bus is populated with a bound driver at apply
> time.
> 
> The notifier machinery is built only for CONFIG_OF_DYNAMIC kernels:
> without it no reconfig notifications exist and
> of_reconfig_notifier_register() is a stub returning -EINVAL, so an
> unconditional registration would fail the initcall on every
> non-dynamic DT kernel.
> 
> 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]

This is not a contest who can ask AI to create the longest commit msg
ever.

You need to configure your LLM properly to Linux kernel coding style.

Best regards,
Krzysztof