[PATCH v2 0/2] ivepatch: Fix several bugs found during replace set implementation

[PATCH v2 0/2] ivepatch: Fix several bugs found during replace set implementation

[Yafang Shao]

These fixes were found while implementing the replace set series [0].
They are independent of that series and can be applied separately.

Link: https://lore.kernel.org/live-patching/20260809091954.22930-1-laoar.shao@gmail.com [0]

Changes:
v1->v2:
- minor improvements and acked-by (Song)
- fix issues reported by sashiko

v1: https://lore.kernel.org/live-patching/20260813030408.9761-1-laoar.shao@gmail.com/

Yafang Shao (2):
  livepatch: Fix wrong index in funcs cleanup error path
  livepatch: Fix UAF of unregistered patch kobjects

 include/linux/livepatch.h |  6 +++++
 kernel/livepatch/core.c   | 50 ++++++++++++++++++++++++++++++++++++---
 scripts/livepatch/init.c  |  2 +-
 3 files changed, 54 insertions(+), 4 deletions(-)

-- 
2.52.0

[PATCH v2 1/2] livepatch: Fix wrong index in funcs cleanup error path

[Yafang Shao]

In the object allocation loop, when kzalloc() for funcs fails, the
cleanup loop uses `objs[i].funcs` instead of `objs[j].funcs`. Since
`objs[i].funcs` is still NULL at that point, it repeatedly calls
kfree(NULL) and leaks all previously allocated funcs arrays.

Fixes: 59adee07b568 ("livepatch/klp-build: Add stub init code for livepatch modules")
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Acked-by: Song Liu <song@kernel.org>
---
 scripts/livepatch/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/livepatch/init.c b/scripts/livepatch/init.c
index f14d8c8fb35f..16aff8f736eb 100644
--- a/scripts/livepatch/init.c
+++ b/scripts/livepatch/init.c
@@ -51,7 +51,7 @@ static int __init livepatch_mod_init(void)
 		if (!funcs) {
 			ret = -ENOMEM;
 			for (int j = 0; j < i; j++)
-				kfree(objs[i].funcs);
+				kfree(objs[j].funcs);
 			goto err_free_objs;
 		}
 
-- 
2.52.0

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

[Yafang Shao]

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). 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.

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.

Dynamic objects and nop functions are excluded from the count because
they are freed by their release callbacks (which call kfree) and only
exist when the patch kobject was successfully added to sysfs. For
patches fully added to sysfs, the kobject parent-child reference chain
ensures the patch kobject is released last, so kobj_pending naturally
reaches zero when the patch kobject is released. For patches with
partial initialization failures, kobj_pending ensures all un-added
kobject releases are tracked regardless of release order.

The release callbacks use early return after kfree() for dynamic/nop
kobjects to avoid dereferencing freed memory. Static kobjects are not
freed by their release callbacks, so accessing obj->patch or
func->obj->patch is safe. Furthermore, static functions always belong
to static objects, so func->obj is never freed by a release callback.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/live-patching/20260809091954.22930-1-laoar.shao@gmail.com/
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 include/linux/livepatch.h |  6 +++++
 kernel/livepatch/core.c   | 50 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index ba9e3988c07c..6375150c2369 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -33,6 +33,7 @@
  * @kobj:	kobject for sysfs resources
  * @node:	list node for klp_object func_list
  * @stack_node:	list node for klp_ops func_stack list
+ * @obj:	back pointer to the owning object
  * @old_size:	size of the old function
  * @new_size:	size of the new function
  * @nop:        temporary patch to use the original code again; dyn. allocated
@@ -72,6 +73,7 @@ struct klp_func {
 	struct kobject kobj;
 	struct list_head node;
 	struct list_head stack_node;
+	struct klp_object *obj;
 	unsigned long old_size, new_size;
 	bool nop;
 	bool patched;
@@ -86,6 +88,7 @@ struct klp_func {
  * @kobj:	kobject for sysfs resources
  * @func_list:	dynamic list of the function entries
  * @node:	list node for klp_patch obj_list
+ * @patch:	back pointer to the owning patch
  * @mod:	kernel module associated with the patched object
  *		(NULL for vmlinux)
  * @dynamic:    temporary object for nop functions; dynamically allocated
@@ -101,6 +104,7 @@ struct klp_object {
 	struct kobject kobj;
 	struct list_head func_list;
 	struct list_head node;
+	struct klp_patch *patch;
 	struct module *mod;
 	bool dynamic;
 	bool patched;
@@ -127,6 +131,7 @@ struct klp_state {
  * @list:	list node for global list of actively used patches
  * @kobj:	kobject for sysfs resources
  * @obj_list:	dynamic list of the object entries
+ * @kobj_pending: number of kobjects awaiting release
  * @enabled:	the patch is enabled (but operation may be incomplete)
  * @forced:	was involved in a forced transition
  * @free_work:	patch cleanup from workqueue-context
@@ -140,6 +145,7 @@ struct klp_patch {
 	bool replace;
 
 	/* internal */
+	atomic_t kobj_pending;
 	struct list_head list;
 	struct kobject kobj;
 	struct list_head obj_list;
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index a240d1144e89..b5e8242b0877 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -635,7 +635,8 @@ static void klp_kobj_release_patch(struct kobject *kobj)
 	struct klp_patch *patch;
 
 	patch = container_of(kobj, struct klp_patch, kobj);
-	complete(&patch->finish);
+	if (atomic_dec_and_test(&patch->kobj_pending))
+		complete(&patch->finish);
 }
 
 static const struct kobj_type klp_ktype_patch = {
@@ -646,12 +647,19 @@ static const struct kobj_type klp_ktype_patch = {
 
 static void klp_kobj_release_object(struct kobject *kobj)
 {
+	struct klp_patch *patch;
 	struct klp_object *obj;
 
 	obj = container_of(kobj, struct klp_object, kobj);
 
-	if (obj->dynamic)
+	if (obj->dynamic) {
 		klp_free_object_dynamic(obj);
+		return;
+	}
+
+	patch = obj->patch;
+	if (atomic_dec_and_test(&patch->kobj_pending))
+		complete(&patch->finish);
 }
 
 static const struct kobj_type klp_ktype_object = {
@@ -662,12 +670,19 @@ static const struct kobj_type klp_ktype_object = {
 
 static void klp_kobj_release_func(struct kobject *kobj)
 {
+	struct klp_patch *patch;
 	struct klp_func *func;
 
 	func = container_of(kobj, struct klp_func, kobj);
 
-	if (func->nop)
+	if (func->nop) {
 		klp_free_func_nop(func);
+		return;
+	}
+
+	patch = func->obj->patch;
+	if (atomic_dec_and_test(&patch->kobj_pending))
+		complete(&patch->finish);
 }
 
 static const struct kobj_type klp_ktype_func = {
@@ -737,9 +752,35 @@ 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);
 
+	/*
+	 * Count all static kobjects (patch + objects + funcs) so that
+	 * klp_free_patch_finish() can wait for all their releases.
+	 *
+	 * Dynamic objects and nop funcs are excluded because they are
+	 * freed by their release callbacks and only exist when the patch
+	 * kobject was successfully added to sysfs.  Static kobjects may
+	 * or may not have been added to sysfs (e.g. if klp_init_object()
+	 * failed after kobject_add() for the patch succeeded).  Counting
+	 * all of them ensures delayed releases are tracked regardless.
+	 */
+	atomic_set(&patch->kobj_pending, 1);
+	klp_for_each_object(patch, obj) {
+		if (obj->dynamic)
+			continue;
+		atomic_inc(&patch->kobj_pending);
+		klp_for_each_func(obj, func) {
+			if (func->nop)
+				continue;
+			atomic_inc(&patch->kobj_pending);
+		}
+	}
+
 	klp_free_objects(patch);
 }
 
@@ -944,6 +985,7 @@ static void klp_init_func_early(struct klp_object *obj,
 {
 	kobject_init(&func->kobj, &klp_ktype_func);
 	list_add_tail(&func->node, &obj->func_list);
+	func->obj = obj;
 }
 
 static void klp_init_object_early(struct klp_patch *patch,
@@ -952,6 +994,7 @@ static void klp_init_object_early(struct klp_patch *patch,
 	INIT_LIST_HEAD(&obj->func_list);
 	kobject_init(&obj->kobj, &klp_ktype_object);
 	list_add_tail(&obj->node, &patch->obj_list);
+	obj->patch = patch;
 }
 
 static void klp_init_patch_early(struct klp_patch *patch)
@@ -962,6 +1005,7 @@ static void klp_init_patch_early(struct klp_patch *patch)
 	INIT_LIST_HEAD(&patch->list);
 	INIT_LIST_HEAD(&patch->obj_list);
 	kobject_init(&patch->kobj, &klp_ktype_patch);
+	atomic_set(&patch->kobj_pending, 0);
 	patch->enabled = false;
 	patch->forced = false;
 	INIT_WORK(&patch->free_work, klp_free_patch_work_fn);
-- 
2.52.0

Re: [PATCH v2 0/2] ivepatch: Fix several bugs found during replace set implementation

[Petr Mladek]

On Sun 2026-08-16 17:04:40, Yafang Shao wrote:
> These fixes were found while implementing the replace set series [0].
> They are independent of that series and can be applied separately.
> 
> Link: https://lore.kernel.org/live-patching/20260809091954.22930-1-laoar.shao@gmail.com [0]
> 
> Changes:
> v1->v2:
> - minor improvements and acked-by (Song)
> - fix issues reported by sashiko
> 
> v1: https://lore.kernel.org/live-patching/20260813030408.9761-1-laoar.shao@gmail.com/
> 
> Yafang Shao (2):
>   livepatch: Fix wrong index in funcs cleanup error path

This patch is against scripts/livepatch/init.c which is part of
klp-build framework. The klp-build-related changes usually
go via the "tip" tree by Josh.

>   livepatch: Fix UAF of unregistered patch kobjects

This patch is against the kernel/livepatch/* code. These changes
usually go via the "livepatching" tree by me.

Honestly, I am not sure how to handle this situation.
Josh?

Anyway, the 1st patch seems to be ready to go.
The 2nd patch might need some more love. I am going to comment on it...

Best Regards,
Petr

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

[Petr Mladek]

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

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

[Yafang Shao]

On Wed, Aug 19, 2026 at 7:18 PM Petr Mladek <pmladek@suse.com> wrote:
>
> 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?

right

>
> 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.

correct

>
> > 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.

good idea

>
> 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);
>  }

It appears that klp_init_object_early() also initializes non-dynamic
objects, right?
Therefore, we should call kobject_put() unconditionally.

 static void klp_kobj_release_object(struct kobject *kobj)
 {
        struct klp_object *obj;
+       struct klp_patch *patch;

        obj = container_of(kobj, struct klp_object, kobj);
+       patch = obj->patch;

        if (obj->dynamic)
                klp_free_object_dynamic(obj);
+
+       kobject_put(&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);
>  }

Similarly, klp_init_func_early() initializes the nop function, so
kobject_put() must be called unconditionally.

 static void klp_kobj_release_func(struct kobject *kobj)
 {
        struct klp_func *func;
+       struct klp_object *obj;

        func = container_of(kobj, struct klp_func, kobj);
+       obj = func->obj;

        if (func->nop)
                klp_free_func_nop(func);
+
+       kobject_put(&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.

-- 
Regards
Yafang

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

[Petr Mladek]

On Wed 2026-08-19 20:37:17, Yafang Shao wrote:
> On Wed, Aug 19, 2026 at 7:18 PM Petr Mladek <pmladek@suse.com> wrote:
> >
> > 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?
> 
> right
> 
> >
> > 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.
> 
> correct
> 
> >
> > > 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.
> 
> good idea
> 
> >
> > 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);
> >  }
> 
> It appears that klp_init_object_early() also initializes non-dynamic
> objects, right?

I was a bit confused by the sentence. The dynamic objects
do not exist when klp_init_object_early() is called in
klp_init_patch_early().

But I see that it is called also in klp_alloc_object_dynamic().

> Therefore, we should call kobject_put() unconditionally.

Great catch. Yes, we should call it unconditionally.

>  static void klp_kobj_release_object(struct kobject *kobj)
>  {
>         struct klp_object *obj;
> +       struct klp_patch *patch;
> 
>         obj = container_of(kobj, struct klp_object, kobj);
> +       patch = obj->patch;
> 
>         if (obj->dynamic)
>                 klp_free_object_dynamic(obj);
> +
> +       kobject_put(&patch->kobj);
>  }

Looks good. Same with klp_kobj_release_func().

Best Regards,
Petr