Re: [PATCH v3 1/4] of: incrementally update /aliases lookup on reconfig notifications

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The deliberate memory leak of the `/aliases` node reference during `OF_RECONFIG_DETACH_NODE` breaks overlay revert and causes an unbounded memory leak.
--

--- Patch [1]: [PATCH v3 1/4] of: incrementally update /aliases lookup on reconfig notifications ---
commit a185916382b462aafe7d63399b28b61ce3a88e5b
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

of: incrementally update /aliases lookup on reconfig notifications

This commit adds an OF reconfig notifier to dynamically update the
/aliases lookup list when nodes or properties change. It handles both
direct devicetree modifications and overlay apply/revert paths, protecting
access to the list with a dedicated aliases_mutex.

Link: https://lore.kernel.org/lkml/1435675876-2159-1-git-send-email-geert+renesas@glider.be/ [1]
Link: https://lore.kernel.org/lkml/20150630172131.D4E6CC4041A@trevor.secretlab.ca/ [2]
Link: https://lore.kernel.org/lkml/1435675876-2159-2-git-send-email-geert+renesas@glider.be/ [3]
Link: https://lore.kernel.org/lkml/1435675876-2159-3-git-send-email-geert+renesas@glider.be/ [4]
Link: https://lore.kernel.org/lkml/1435675876-2159-4-git-send-email-geert+renesas@glider.be/ [5]

> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 6e7a42dedad3f..4e34c65a8f9f0 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c

[ ... ]

> +static int of_aliases_reconfig_notifier(struct notifier_block *nb,
> +					unsigned long action, void *arg)
> +{
> +	struct of_reconfig_data *rd = arg;
> +
> +	/* of_aliases may still be NULL when an overlay creates the node */
> +	if (!rd->dn || !rd->dn->parent ||
> +	    !of_node_is_root(rd->dn->parent) ||
> +	    !of_node_name_eq(rd->dn, "aliases"))
> +		return NOTIFY_DONE;
> +
> +	mutex_lock(&aliases_mutex);
> +	switch (action) {
> +	case OF_RECONFIG_ATTACH_NODE:
> +		if (!of_aliases)
> +			of_aliases = of_node_get(rd->dn);
> +		break;
> +	case OF_RECONFIG_DETACH_NODE:
> +		of_aliases_forget_all();
> +		/* keep the ATTACH reference: lockless readers may hold the node */
> +		if (of_aliases == rd->dn)
> +			of_aliases = NULL;

[Severity: High]
Does skipping of_node_put() here cause a memory leak of the device node
and its properties when reverting overlays?

During the ATTACH_NODE notification, the notifier takes a reference on the
node using of_node_get(). When the overlay is later reverted, the DETACH_NODE
handler clears the pointer but intentionally omits of_node_put().

When the overlay changeset is subsequently destroyed,
__of_changeset_entry_destroy() checks the node's reference count. Because this
notifier leaked a reference, the count remains elevated. 

This elevated count causes the destruction process to print a memory leak error
and skip setting the OF_OVERLAY_FREE_CSET flag.

Consequently, of_node_release() will refuse to free the node's memory, leaking
the node on every overlay apply/revert cycle.

> +		break;
> +	case OF_RECONFIG_ADD_PROPERTY:
> +		of_alias_create(rd->prop, alias_alloc, true);
> +		break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-nh-of-alias-overlay-v3-0-7001028fe2f5@nexthop.ai?part=1