Re: [PATCH v6 05/10] of/overlay: put property on deadprops only after changeset add succeeds

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Leaked fragment target and overlay node references when init_overlay_changeset() fails mid-iteration.
--

--- Patch [5]: [PATCH v6 05/10] of/overlay: put property on deadprops only after changeset add succeeds ---
commit 32c9bbaae9841195a053f81f6d3eb35116c97a19
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

of/overlay: put property on deadprops only after changeset add succeeds

This commit fixes a double-free issue in add_changeset_property().
It ensures a new property is only linked into the target node's deadprops
list after it has been successfully added to the changeset.

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

[ ... ]

While reviewing this file, I noticed a pre-existing issue nearby:

drivers/of/overlay.c:init_overlay_changeset() {
	...
	cnt = 0;
	for_each_child_of_node(ovcs->overlay_root, node) {
		overlay_node = of_get_child_by_name(node, "__overlay__");
		if (!overlay_node)
			continue;

		fragment = &fragments[cnt];
		fragment->overlay = overlay_node;
		fragment->target = find_target(node, target_base);
		if (!fragment->target) {
			of_node_put(fragment->overlay);
			ret = -EINVAL;
			of_node_put(node);
			goto err_out;
		}

		cnt++;
	}
	...
}

[Severity: High]
This is a pre-existing issue, but does this error path leak node references?

If find_target() fails for a later fragment, we jump to err_out without
updating ovcs->count.

When free_overlay_changeset() is later called to clean up, it relies on
ovcs->count to free the fragments:

drivers/of/overlay.c:free_overlay_changeset() {
	...
	for (i = 0; i < ovcs->count; i++) {
		of_node_put(ovcs->fragments[i].target);
		of_node_put(ovcs->fragments[i].overlay);
	}
	...
}

Because ovcs->count was never updated before the error jump in
init_overlay_changeset(), won't this skip cleaning up the successfully
processed fragments and leak their target and overlay node references?

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