Re: [PATCH v7 for-next 2/8] livepatch: Call klp_init_patch_early() earlier

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Potential use-after-free of kobject delayed work structs during error cleanup if CONFIG_DEBUG_KOBJECT_RELEASE is enabled.
--

--- Patch [2]: [PATCH v7 for-next 2/8] livepatch: Call klp_init_patch_early() earlier ---
commit b5f9135d749ffc4d6ea6c6a68a0fc6489e7a1cbf
Author: Yafang Shao <laoar.shao@gmail.com>

livepatch: Call klp_init_patch_early() earlier
    
Invoke klp_init_patch_early() during early initialization to avoid mixing
for_each_() and for_each_*_static() variants later in the code.

> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
[ ... ]
> @@ -1152,6 +1152,14 @@ int klp_enable_patch(struct klp_patch *patch)
>  
>  	klp_init_patch_early(patch);
>  
> +	if (!klp_is_patch_compatible(patch)) {
> +		pr_err("Livepatch patch (%s) is not compatible with the already installed livepatches.\n",
> +			patch->mod->name);
> +		ret = -EINVAL;
> +		goto err;
> +	}

[Severity: High]
This is a pre-existing issue, but does this error path expose a use-after-free
of kobject delayed work structs when CONFIG_DEBUG_KOBJECT_RELEASE is enabled?

Because kobject_add() has not yet run (it runs later in klp_init_patch()), the
child kobjects (obj and func) do not have their parent pointers set to
patch->kobj during this cleanup path.

When the error path jumps to err and calls klp_free_patch_start(), it drops
references to the child kobjects. Since their parent pointers aren't set,
putting the child kobjects does not elevate or pin the parent patch->kobj. 

Consequently, the wait_for_completion() in klp_free_patch_finish() only waits
for the patch kobject to finish releasing, but not the child kobjects:

kernel/livepatch/core.c:klp_free_patch_finish() {
	...
	kobject_put(&patch->kobj);
	wait_for_completion(&patch->finish);

	/* Put the module after the last access to struct klp_patch. */
	if (!patch->forced)
		module_put(patch->mod);
}

This allows the module memory to be completely freed while the delayed work
for the child obj and func kobjects (which reside in the now-freed module
memory) might still be queued. When the workqueue executes them, it results in
a use-after-free.

While this issue already existed for klp_init_patch() failures, moving
klp_is_patch_compatible() after klp_init_patch_early() makes it trivially
reachable by simply providing an incompatible livepatch.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825114641.80452-1-laoar.shao@gmail.com?part=2