Re: [PATCH v4 4/6] 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] Double-free and use-after-free of `new_prop` on the `of_changeset_add_property()` error path.
--

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

of/overlay: return ERR_PTR from dup_and_fixup_symbol_prop()

dup_and_fixup_symbol_prop() previously returned NULL for malformed input,
unresolved paths, and allocation failures, which the caller collapsed into
-ENOMEM. This patch changes the function to return an ERR_PTR instead.
The caller is updated to propagate the specific errno.

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

[ ... ]

> @@ -350,6 +354,8 @@ static int add_changeset_property(struct overlay_changeset *ovcs,
>  		if (prop)
>  			return -EINVAL;
>  		new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop);
> +		if (IS_ERR(new_prop))
> +			return PTR_ERR(new_prop);
>  	} else {
>  		new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL);
>  	}

[Severity: High]
This is a pre-existing issue, but could this code path lead to a double-free
on the error path for add_changeset_property()?

Looking further down in add_changeset_property(), if we are not in a livetree,
new_prop is linked into the deadprops list:

	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 of_changeset_add_property() fails (for example, due to memory exhaustion),
ret is non-zero, and the function frees new_prop at the bottom:

	if (ret)
		__of_prop_free(new_prop);

Does this leave the freed new_prop still attached to target->np->deadprops?

When the overlay application aborts and the unattached dynamically allocated
device node is cleaned up via of_node_release(), it iterates over the
deadprops list and calls __of_prop_free() on each property again. Can this
result in a use-after-free and double-free of the property?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-nh-of-alias-overlay-v4-0-8ad097e31e36@nexthop.ai?part=4