Re: [PATCH v6 07/10] of/overlay: don't create "//" paths for fragments targeting the root

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Node reference leak for initialized fragments if find_target() fails in init_overlay_changeset().
--

--- Patch [7]: [PATCH v6 07/10] of/overlay: don't create "//" paths for fragments targeting the root ---
commit 160c0459de49f61664940cc0b23bceacbf1b3082
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

of/overlay: don't create "//" paths for fragments targeting the root
    
dup_and_fixup_symbol_prop() rewrites a symbol value by replacing its
"/fragment/__overlay__" prefix with the fragment's target path. When
the fragment targets the root node the target path is "/" and the
result starts with "//", which __of_find_node_by_full_path() cannot
resolve.
    
Drop the target path when it is the root and the tail is non-empty. A
value naming the fragment root itself (empty tail) keeps the "/".

> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 9b9f198a1d70a..89a9fe6d6e786 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -256,6 +256,9 @@ static struct property *dup_and_fixup_symbol_prop(
>  	if (!target_path)
>  		return NULL;
>  	target_path_len = strlen(target_path);
> +	/* a root target renders as "/"; drop it to avoid "//" results */
> +	if (target_path_len == 1 && target_path[0] == '/' && path_tail_len)
> +		target_path_len = 0;
>  
>  	new_prop = kzalloc_obj(*new_prop);
>  	if (!new_prop)

[ ... ]

> @@ -804,13 +804,13 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs,
>  	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;
>  		}

[Severity: High]
This is a pre-existing issue, but does this jump to err_out leak device
node references if previous fragments were already successfully populated?

If a subsequent fragment's find_target() fails (e.g., due to an invalid target
path in the overlay), the function jumps to err_out without updating
ovcs->count.

Because ovcs->count remains 0, the cleanup function free_overlay_changeset()
will skip its loop and fail to call of_node_put() for the previously
successfully resolved fragment nodes.

Could ovcs->count be updated to cnt before jumping to err_out so that
free_overlay_changeset() can clean up the previously populated elements?

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