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

On Sun 2026-08-16 17:04:42, Yafang Shao wrote:

When klp_enable_patch() fails after klp_init_patch_early() has run,
the error path calls klp_free_patch_start() and klp_free_patch_finish().
The former drops the references of all object and function kobjects via
klp_free_objects(), the latter drops the patch kobject reference and
waits for the patch kobject release only:

    klp_free_patch_finish():
        kobject_put(&patch->kobj);
        wait_for_completion(&patch->finish);

With CONFIG_DEBUG_KOBJECT_RELEASE enabled, kobject_put() does not
release the kobject synchronously but schedules a delayed release with
a random delay of up to 4 seconds (see kobject_release() in
lib/kobject.c).

Yes.

Because klp_free_patch_finish() only waits for the
patch kobject release, it may return while object and function kobject
releases are still pending. The caller can then unload the livepatch
module, which frees the klp_object and klp_func structures. The delayed
kobject release callbacks later access this freed memory in
kobject_cleanup(), resulting in a use-after-free.

This issue can occur in two scenarios:

1. The patch kobject was never added to sysfs (e.g., klp_init_patch()
   failed at kobject_add()). All child kobjects were only initialized
   via kobject_init() but never added to sysfs. They do not hold
   references to the patch kobject, so the patch kobject can be
   released independently, unblocking patch->finish before the child
   releases complete.

2. The patch kobject was added to sysfs, but a subsequent operation
   such as klp_add_nops() or klp_init_object() failed. Some child
   kobjects were initialized but not yet added to sysfs. These
   un-added children do not hold references to the patch kobject
   either, so the same race can occur.

In short, this says that the races might happen when some kobjects
were not added into sysfs. Am I right, please?

I agree. My undestading:

The klp_kobj_release_*() callbacks are called by kobject_cleanup()
which calls kobject_put(parent) as the last step. It should make sure
that:

  + klp_kobj_release_patch() is scheduled/called only when
    klp_kobj_release_object() has been called for all patch->objs.

  + klp_kobj_release_object() is scheduled/called only when
    klp_kobj_release_func() has been called for all obj->funcs.

But it works only when "kobj->parent" is set and
"parent->kref" has been incremented for each child before.

This is true only when kobject_add() is called for all all used
kobjects. But it is not guaranteed when any klp_init_*() failed.

Fix this by tracking all static kobject releases with a per-patch
atomic counter (kobj_pending). klp_free_patch_start() counts the
patch kobject plus all static object and function kobjects.
klp_free_patch_finish() waits until kobj_pending reaches zero,
ensuring all kobject releases have completed before the module is
unloaded.

I think that we do not need an extra couter. We might use
the existing kobj->kref. We just need to explicitely
increment/decrement it.

I mean something like:

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 28d15ba58a26..023f666ddcc4 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c

@@ -652,6 +652,8 @@ static void klp_kobj_release_object(struct kobject *kobj)
 
 	if (obj->dynamic)
 		klp_free_object_dynamic(obj);
+	else
+		kobject_put(&obj->patch.kobj);
 }
 
 static const struct kobj_type klp_ktype_object = {

@@ -668,6 +670,8 @@ static void klp_kobj_release_func(struct kobject *kobj)
 
 	if (func->nop)
 		klp_free_func_nop(func);
+	else
+		kobject_put(&func->obj.kobj);
 }
 
 static const struct kobj_type klp_ktype_func = {

@@ -946,6 +950,7 @@ static void klp_init_func_early(struct klp_object *obj,
 				struct klp_func *func)
 {
 	kobject_init(&func->kobj, &klp_ktype_func);
+	kobject_get(&obj->kobj);
 	list_add_tail(&func->node, &obj->func_list);
 }
 

@@ -954,6 +959,7 @@ static void klp_init_object_early(struct klp_patch *patch,
 {
 	INIT_LIST_HEAD(&obj->func_list);
 	kobject_init(&obj->kobj, &klp_ktype_object);
+	kobject_get(&patch->kobj);
 	list_add_tail(&obj->node, &patch->obj_list);
 }

We really would need to add the back references (obj->patch,
func->obj) because we could not rely on kobj->parent. It is
set only when kobject_add() was called...

That said, I doubt that livepatching is the only subsystem using
kobjects in static structures. It might make sense to handle
this on the kobject API level. I mean to add a kobject() API
which would just set kobj->parent and increment kobj->kref
and can't fail. But it seems to be against the existing philosophy
of the kobject API. So, we might need the workaround after all.

Best Regards,
Petr