Re: [PATCH RFC 3/4] of/overlay: rewrite /aliases path values to live-tree paths

[sashiko-bot@kernel.org]

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

New issues:
- [High] Fallback to `__of_prop_dup()` for `/aliases` masks ENOMEM errors and bypasses null-termination safety checks, causing silent corruption and potential OOB reads.

Pre-existing issues:
- [Critical] Dangling pointer in target->np->deadprops leads to use-after-free and double-free if of_changeset_add_property() fails.
--

--- Patch [3]: [PATCH RFC 3/4] of/overlay: rewrite /aliases path values to live-tree paths ---
commit 8741f5ffd2983da599c7be77cbf8c1af8325d4be
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

of/overlay: rewrite /aliases path values to live-tree paths

/aliases entries added by an overlay reference labeled nodes inside
the overlay via '&label' in the .dtso. dtc renders those references
as string paths at compile time, but the paths encode the overlay's
internal fragment layout (e.g. "/fragment@1/__overlay__/fpga@0/i2c@40000")
rather than the location where the node will live after apply.

> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 654a70d5cb075..338786cef17ab 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -350,6 +350,19 @@ static int add_changeset_property(struct overlay_changeset *ovcs,
>  		if (prop)
>  			return -EINVAL;
>  		new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop);
> +	} else if (target->np->parent &&
> +		   of_node_is_root(target->np->parent) &&
> +		   of_node_name_eq(target->np, "aliases")) {
> +		/*
> +		 * /aliases property values are string paths in the same
> +		 * "/fragment@N/__overlay__/..." form as /__symbols__.
> +		 * Rewrite them to live-tree paths so of_alias_get_id() can
> +		 * resolve them; fall back to a raw dup for non-fragment
> +		 * values (e.g. legacy string aliases like "ttyS0").
> +		 */
> +		new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop);
> +		if (!new_prop)
> +			new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL);

[Severity: High]
If dup_and_fixup_symbol_prop() fails because the string is not
null-terminated, does this fallback to __of_prop_dup() end up copying the
malformed string?

Because dup_and_fixup_symbol_prop() can return NULL for both memory
allocation failures and structural errors like non-null-terminated strings,
falling back to a raw duplicate here preserves the non-null-terminated
string.

Later, the reconfig notifier can trigger of_alias_create() which attempts to
read the property:

drivers/of/base.c:of_alias_create() {
    ...
	if (owned)
		np = of_find_node_by_path(pp->value);
    ...
}

Could this lead to an out-of-bounds read when of_find_node_by_path() treats
the non-null-terminated property value as a C string?

Additionally, does this mask memory allocation failures from
dup_and_fixup_symbol_prop() by falling back to a raw duplicate, injecting
unresolved paths directly into the live tree instead of failing the overlay
application?

>  	} else {
>  		new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL);
>  	}

[Severity: Critical]
This is a pre-existing issue, but while reviewing add_changeset_property(), I
noticed a problem further down in the same function.

When a property is added to a newly created node, it is linked into the
node's deadprops list before the changeset operation:

drivers/of/overlay.c:add_changeset_property() {
    ...
	if (!prop) {
		if (!target->in_livetree) {
			new_prop->next = target->np->deadprops;
			target->np->deadprops = new_prop;
		}
		ret = of_changeset_add_property(&ovcs->cset, target->np,
						new_prop);
    ...
	if (ret)
		__of_prop_free(new_prop);
	return ret;
}

If of_changeset_add_property() fails, perhaps due to an allocation error,
__of_prop_free() is called on new_prop, but new_prop is not unlinked from
target->np->deadprops.

Will this leave a dangling pointer in the deadprops list, causing a double
free later when the changeset is aborted and of_node_release() calls
property_list_free() on node->deadprops?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-nh-of-alias-overlay-v1-0-f1e5d9889b30@nexthop.ai?part=3