Re: [RFC PATCH v4 2/4] livepatch: Call klp_init_patch_early() earlier

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Moving klp_init_patch_early() before early failure checks violates the kobject API lifecycle and introduces missing cleanup on error paths.
--

--- Patch [2]: [RFC PATCH v4 2/4] livepatch: Call klp_init_patch_early() earlier ---
commit 6b8df20b82104d4c61053ebea3721f1d77891456
Author: Yafang Shao <[email protected]>

livepatch: Call klp_init_patch_early() earlier

This commit moves the invocation of klp_init_patch_early() earlier in the
livepatch initialization process. This prepares the code for an upcoming
patch by avoiding mixing iteration variants later in the sequence.

> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index e07c41a0f3ac8..c98cbcf59566d 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -1135,6 +1135,8 @@ int klp_enable_patch(struct klp_patch *patch)
>  
>  	mutex_lock(&klp_mutex);
>  
> +	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);
> @@ -1147,8 +1149,6 @@ int klp_enable_patch(struct klp_patch *patch)
>  		return -ENODEV;
>  	}

[Severity: Medium]
Since klp_init_patch_early() initializes kobjects for the patch, does
returning directly on compatibility or try_module_get() failures leak
these resources?

The kobject API requires that once kobject_init() is called, kobject_put()
must be used to release the object. Because klp_init_patch_early() is now
called before these checks, if we hit the early return paths in
klp_enable_patch(), we bypass cleaning up the initialized kobjects.

Also, would simply changing these early returns to jump to the err label
cause issues with klp_free_patch_finish()?

kernel/livepatch/core.c:klp_enable_patch() {
    ...
err:
	klp_free_patch_start(patch);
	mutex_unlock(&klp_mutex);
	klp_free_patch_finish(patch);
	return ret;
}

It looks like klp_free_patch_finish() calls module_put(patch->mod)
unconditionally. If try_module_get() failed, jumping to the err label
might underflow the module reference count.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2