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

On Tue, Aug 4, 2026 at 3:03 PM <[email protected]> wrote:
>
> 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.

Good point. It seems we need the following additional change:

--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -1168,19 +1168,20 @@ int klp_enable_patch(struct klp_patch *patch)

        mutex_lock(&klp_mutex);

+       if (!try_module_get(patch->mod)) {
+               mutex_unlock(&klp_mutex);
+               return -ENODEV;
+       }
+
        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);
-               mutex_unlock(&klp_mutex);
-               return -EINVAL;
+               ret = -EINVAL;
+               goto err;
        }

-       if (!try_module_get(patch->mod)) {
-               mutex_unlock(&klp_mutex);
-               return -ENODEV;
-       }

        ret = klp_init_patch(patch);
        if (ret)

-- 
Regards
Yafang