Re: [PATCH v6 04/10] of/overlay: look up absolute target-paths absolutely

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Medium] Pre-existing memory leak of target and overlay device node references occurs on the error paths of `init_overlay_changeset()`.
--

--- Patch [4]: [PATCH v6 04/10] of/overlay: look up absolute target-paths absolutely ---
commit 28d3c18ff8d649fd8af8d895999493b24c338fa1
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

of/overlay: look up absolute target-paths absolutely

This commit updates find_target() to resolve non-empty target-path properties
as absolute paths from the live-tree root rather than appending them to the
target base. Empty target-path values still resolve to the target base itself.
The documentation for init_overlay_changeset() and of_overlay_apply() is
updated to reflect this new behavior.

> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 08d5351746be6..74aea704835ac 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c

[ ... ]

> @@ -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

[Severity: Medium]
This is a pre-existing issue, but does init_overlay_changeset() leak
device node references when an error occurs during fragment processing?

Looking at init_overlay_changeset(), if find_target() fails (or if the
__symbols__ target is missing), it jumps to err_out without updating
ovcs->count:

		fragment->target = find_target(node, target_base);
		if (!fragment->target) {
			of_node_put(fragment->overlay);
			ret = -EINVAL;
			of_node_put(node);
			goto err_out;
		}

Because ovcs->count is only updated to the successfully processed count
at the very end of the function, it remains 0 during the error return.

When the caller subsequently runs free_overlay_changeset() to clean up the
partially initialized changeset, its cleanup loop is skipped entirely because
ovcs->count is 0:

	for (i = 0; i < ovcs->count; i++) {
		of_node_put(ovcs->fragments[i].target);
		of_node_put(ovcs->fragments[i].overlay);
	}

Would this permanently leak the node references for any fragments that were
successfully processed before the failure occurred?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-nh-of-alias-overlay-v6-0-74f21d440819@nexthop.ai?part=4