Re: [PATCH v4 5/7] of/overlay: return ERR_PTR from dup_and_fixup_symbol_prop()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] When an overlay targets the root node (`/`), `dup_and_fixup_symbol_prop` creates malformed device tree paths with a double slash (`//`), preventing the symbol or alias from being resolved.
--

--- Patch [5]: [PATCH v4 5/7] of/overlay: return ERR_PTR from dup_and_fixup_symbol_prop() ---
commit 5d340ac0a28e3bab97719502d9467e332f065d74
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

of/overlay: return ERR_PTR from dup_and_fixup_symbol_prop()

This commit modifies dup_and_fixup_symbol_prop() to return distinct ERR_PTR
values instead of NULL for different failure cases. It also adds verification
to ensure the value descends through the matched fragment's __overlay__ node
before slicing it.

> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 284c9bc6c9cfc..d4d2591e735ac 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
[ ... ]
> @@ -243,18 +249,27 @@ static struct property *dup_and_fixup_symbol_prop(
[ ... ]
>  	path_tail = path + overlay_name_len;
>  	path_tail_len = strlen(path_tail);
>  
>  	target_path = kasprintf(GFP_KERNEL, "%pOF", fragment->target);
>  	if (!target_path)
> -		return NULL;
> +		return ERR_PTR(-ENOMEM);
>  	target_path_len = strlen(target_path);
>  

[Severity: High]
This is a pre-existing issue, but since the commit message notes this helper
is being prepared for /aliases, could the concatenation of target_path and
path_tail further down in dup_and_fixup_symbol_prop() create invalid paths
when the overlay targets the root node?

When fragment->target is the root node, target_path evaluates to "/". If
path_tail references a fragment-internal path and starts with a slash
(e.g., "/my_node"), unconditionally concatenating them without deduplicating
the connecting slash creates a path like "//my_node":

    memcpy(new_prop->value, target_path, target_path_len);
    memcpy(new_prop->value + target_path_len, path_tail, path_tail_len);

Because __of_find_node_by_full_path() fails on empty path components, any
alias or symbol in an overlay targeting the root node will silently fail to
resolve. Does this need to be addressed so drivers relying on these aliases
do not fail to probe?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-nh-of-alias-overlay-v4-0-fc96a40d2761@nexthop.ai?part=5