Re: [PATCH v4 7/7] of: unittest: cover /aliases updates from overlay apply/revert

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but a guaranteed NULL pointer dereference occurs in the error handling path of `of_overlay_fdt_apply()` when `idr_alloc()` fails.
--

--- Patch [7]: [PATCH v4 7/7] of: unittest: cover /aliases updates from overlay apply/revert ---
commit 546dfcaa5ee21a92732ec54a5aca463b27b6686c
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

of: unittest: cover /aliases updates from overlay apply/revert

Add overlay_alias.dtso plus of_unittest_overlay_alias() to cover the
"aliases inside an overlay" flow end-to-end. The runner applies the
overlay with a non-NULL target_base via of_overlay_fdt_apply() directly.

> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index e255f54f4d760..2f5e4056efae0 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
[ ... ]
> @@ -3475,6 +3475,77 @@ static struct notifier_block of_nb = {
>  	.notifier_call = of_notify,
>  };
>  
> +/* created by cmd_wrap_S_dtbo in scripts/Makefile.dtbs */
> +extern uint8_t __dtbo_overlay_alias_begin[];
> +extern uint8_t __dtbo_overlay_alias_end[];
> +
> +static void __init of_unittest_overlay_alias(void)
> +{
[ ... ]
> +	EXPECT_BEGIN(KERN_INFO,
> +		     "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /aliases/testcase-alias99");
> +
> +	ret = of_overlay_fdt_apply(__dtbo_overlay_alias_begin, size,
> +				   &ovcs_id, base);

[Severity: High]
This is a pre-existing issue, but is there a guaranteed NULL pointer
dereference in the error handling path of of_overlay_fdt_apply() when
idr_alloc() fails?

If idr_alloc() fails (e.g., due to low memory), it returns a negative error
code (like -ENOMEM). This negative value is stored in ovcs->id, and the code
jumps to err_free_ovcs without initializing ovcs->ovcs_list:

drivers/of/overlay.c:of_overlay_fdt_apply() {
	...
	ovcs->id = idr_alloc(&ovcs_idr, ovcs, 1, 0, GFP_KERNEL);
	if (ovcs->id <= 0) {
		ret = ovcs->id;
		goto err_free_ovcs;
	}
	...
}

In free_overlay_changeset(), the check `if (ovcs->id)` evaluates to true
because negative numbers are non-zero. It then calls
list_del(&ovcs->ovcs_list) on a zero-initialized list, dereferencing
NULL->next:

drivers/of/overlay.c:free_overlay_changeset() {
	...
	if (ovcs->id) {
		idr_remove(&ovcs_idr, ovcs->id);
		list_del(&ovcs->ovcs_list);
		ovcs->id = 0;
	}
	...
}

Could this memory allocation failure under load cause a kernel panic during an
overlay apply attempt?

> +
> +	EXPECT_END(KERN_INFO,
> +		   "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /aliases/testcase-alias99");

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