Re: [PATCH 2/2] livepatch: Fix UAF of unregistered patch kobjects

On Thu, Aug 13, 2026 at 11:16 AM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [Critical] The newly added release signaling logic introduces a direct Use-After-Free when processing dynamically allocated objects and NOP functions in the kobject release paths.
>
> Pre-existing issues:
> - [High] The patch fails to fully resolve the original delayed kobject release Use-After-Free if partial initialization fails after the patch kobject has been added, leaving child kobjects un-added.
> --
>
> --- Patch [2]: [PATCH 2/2] livepatch: Fix UAF of unregistered patch kobjects ---
> commit ac38837d810b471eed544191021bf254f8b9e585
> Author: Yafang Shao <[email protected]>
>
> livepatch: Fix UAF of unregistered patch kobjects
>
> When klp_enable_patch() fails after klp_init_patch_early() has run,
> delayed kobject releases can cause use-after-free bugs when the module
> is unloaded. This patch tracks pending kobject releases and waits for
> them before completion.
>
> Link: https://lore.kernel.org/all/[email protected]/
>
> > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > index 28d15ba58a26d..e07b026feeb99 100644
> > --- a/kernel/livepatch/core.c
> > +++ b/kernel/livepatch/core.c
>
> [ ... ]
>
> > @@ -652,6 +653,14 @@ static void klp_kobj_release_object(struct kobject *kobj)
> >
> >       if (obj->dynamic)
> >               klp_free_object_dynamic(obj);
> > +
> > +     /*
> > +      * The object kobject was initialized but never added to sysfs.
> > +      * Signal the release to the owning patch.
> > +      */
> > +     if (!obj->patch->kobj_added &&
> > +         atomic_dec_and_test(&obj->patch->kobj_pending))
> > +             complete(&obj->patch->finish);
> >  }
>
> [Severity: Critical]
> Does this dereference the freed obj pointer?
>
> If obj->dynamic is true, klp_free_object_dynamic(obj) is called, which
> frees the object structure. Immediately after this intentional free, the
> newly added logic dereferences the freed pointer by evaluating
> !obj->patch->kobj_added.

makes sense.
It seems we only fix it for the !obj->dynamic case.

>
> > @@ -668,6 +677,13 @@ static void klp_kobj_release_func(struct kobject *kobj)
> >
> >       if (func->nop)
> >               klp_free_func_nop(func);
> > +     /*
> > +      * The function kobject was initialized but never added to sysfs.
> > +      * Signal the release to the owning patch.
> > +      */
> > +     if (!func->obj->patch->kobj_added &&
> > +         atomic_dec_and_test(&func->obj->patch->kobj_pending))
> > +             complete(&func->obj->patch->finish);
> >  }
>
> [Severity: Critical]
> Similarly here, does this access func after it has been freed?
>
> If func->nop is true, klp_free_func_nop(func) frees the function,
> but the new check immediately dereferences func->obj->patch->kobj_added.
>

makes sense.
We only need to fix it for the !func->nop case.

> [ ... ]
>
> > @@ -737,9 +753,29 @@ static void klp_free_objects_dynamic(struct klp_patch *patch)
> >   */
> >  static void klp_free_patch_start(struct klp_patch *patch)
> >  {
> > +     struct klp_object *obj;
> > +     struct klp_func *func;
> > +
> >       if (!list_empty(&patch->list))
> >               list_del(&patch->list);
> >
> > +     if (!patch->kobj_added) {
>
> [Severity: High]
> This is a pre-existing issue, but does this check leave child kobjects
> unprotected if partial initialization fails after the patch kobject has
> been added?
>
> If klp_init_patch() successfully adds the patch kobject, it sets
> patch->kobj_added = true. If a subsequent operation like klp_add_nops()
> or klp_init_object() fails, klp_free_patch_start() is called on the
> error path.
>
> Because patch->kobj_added is true, this check is bypassed and
> patch->kobj_pending tracking is not initialized. However, child kobjects
> that were initialized early but not yet added to sysfs do not hold a
> reference to patch->kobj. Thus, patch->kobj will be released
> independently, unblocking patch->finish, and allowing the livepatch
> module to be unloaded while un-added child kobjects may still trigger
> their delayed releases and access freed module memory.

Right.
The kobj_added flag is insufficient because it doesn't account
for partial initialization failures where the patch kobject was added
to sysfs but some child kobjects were not. Removing kobj_added and
always counting all static kobjects in kobj_pending handles both the
fully-unadded and partially-added cases uniformly.

-- 
Regards
Yafang