* [PATCH 0/2] livepatch: Fix several bugs found during replace set implementation
@ 2026-08-13 3:04 Yafang Shao
2026-08-13 3:04 ` [PATCH 1/2] livepatch: Fix wrong index in funcs cleanup error path Yafang Shao
2026-08-13 3:04 ` [PATCH 2/2] livepatch: Fix UAF of unregistered patch kobjects Yafang Shao
0 siblings, 2 replies; 5+ messages in thread
From: Yafang Shao @ 2026-08-13 3:04 UTC (permalink / raw)
To: jpoimboe, jikos, mbenes, pmladek, joe.lawrence, song
Cc: live-patching, 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]
Yafang Shao (2):
livepatch: Fix wrong index in funcs cleanup error path
livepatch: Fix UAF of unregistered patch kobjects
include/linux/livepatch.h | 8 ++++++++
kernel/livepatch/core.c | 43 ++++++++++++++++++++++++++++++++++++++-
scripts/livepatch/init.c | 4 ++--
3 files changed, 52 insertions(+), 3 deletions(-)
--
2.52.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] livepatch: Fix wrong index in funcs cleanup error path
2026-08-13 3:04 [PATCH 0/2] livepatch: Fix several bugs found during replace set implementation Yafang Shao
@ 2026-08-13 3:04 ` Yafang Shao
2026-08-13 3:04 ` [PATCH 2/2] livepatch: Fix UAF of unregistered patch kobjects Yafang Shao
1 sibling, 0 replies; 5+ messages in thread
From: Yafang Shao @ 2026-08-13 3:04 UTC (permalink / raw)
To: jpoimboe, jikos, mbenes, pmladek, joe.lawrence, song
Cc: live-patching, 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>
---
scripts/livepatch/init.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/livepatch/init.c b/scripts/livepatch/init.c
index f14d8c8fb35f..77746984aa57 100644
--- a/scripts/livepatch/init.c
+++ b/scripts/livepatch/init.c
@@ -50,8 +50,6 @@ static int __init livepatch_mod_init(void)
funcs = kzalloc(sizeof(struct klp_func) * (nr_funcs + 1), GFP_KERNEL);
if (!funcs) {
ret = -ENOMEM;
- for (int j = 0; j < i; j++)
- kfree(objs[i].funcs);
goto err_free_objs;
}
@@ -81,6 +79,8 @@ static int __init livepatch_mod_init(void)
return klp_enable_patch(patch);
err_free_objs:
+ for (int i = 0; i < nr_objs; i++)
+ kfree(objs[i].funcs);
kfree(objs);
err_free_patch:
kfree(patch);
--
2.52.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] livepatch: Fix UAF of unregistered patch kobjects
2026-08-13 3:04 [PATCH 0/2] livepatch: Fix several bugs found during replace set implementation Yafang Shao
2026-08-13 3:04 ` [PATCH 1/2] livepatch: Fix wrong index in funcs cleanup error path Yafang Shao
@ 2026-08-13 3:04 ` Yafang Shao
2026-08-13 3:16 ` sashiko-bot
1 sibling, 1 reply; 5+ messages in thread
From: Yafang Shao @ 2026-08-13 3:04 UTC (permalink / raw)
To: jpoimboe, jikos, mbenes, pmladek, joe.lawrence, song
Cc: live-patching, Yafang Shao, sashiko-bot
When klp_enable_patch() fails after klp_init_patch_early() has run
(e.g. when klp_init_patch() can't add the patch kobject to sysfs due
to an allocation failure), the error path calls klp_free_patch_start()
and klp_free_patch_finish(). The former drops the references of the object
and function kobjects that were only initialized but never added to sysfs,
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() does not wait for the
object and function kobject releases, it may return while they are
still pending. The caller can then unload the livepatch module, which
frees the klp_object and klp_func structures (either statically
defined in the module core layout or dynamically allocated by the
module init code). The delayed kobject release callbacks later access
this freed memory in kobject_cleanup(), resulting in a use-after-free.
Fix this by making patch->finish wait for the release of all
initialized kobjects that were never added to sysfs. Track the number
of pending kobject releases in struct klp_patch and complete
patch->finish when the last one is released.
For patches whose kobject was already added to sysfs, the existing
behavior is preserved and no counting is needed. In this case, the
kobject parent-child reference chain guarantees the release order:
kobject_add() bumps the parent reference (kobject_get() in
kobject_add_internal()) and the child release drops it again
(kobject_put() in kobject_cleanup()). As a result, the patch kobject
can't be released until all object kobjects are released, which in
turn can't happen until all function kobjects are released. The patch
release therefore always happens after all its sub-kobjects, so
waiting for patch->finish alone is sufficient.
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260809094046.50ED31F000E9@smtp.kernel.org/
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
include/linux/livepatch.h | 8 ++++++++
kernel/livepatch/core.c | 43 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index ba9e3988c07c..1ed0d71e965b 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 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
@@ -73,6 +74,7 @@ struct klp_func {
struct list_head node;
struct list_head stack_node;
unsigned long old_size, new_size;
+ struct klp_object *obj;
bool nop;
bool patched;
bool transition;
@@ -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 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,8 @@ 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
+ * @kobj_added: the patch kobject was added to sysfs
* @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 +146,8 @@ struct klp_patch {
bool replace;
/* internal */
+ atomic_t kobj_pending;
+ bool kobj_added;
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..e0b0ef7aeb29 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 (patch->kobj_added || atomic_dec_and_test(&patch->kobj_pending))
+ complete(&patch->finish);
}
static const struct kobj_type klp_ktype_patch = {
@@ -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);
}
static const struct kobj_type klp_ktype_object = {
@@ -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);
}
static const struct kobj_type klp_ktype_func = {
@@ -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) {
+ /*
+ * The kobjects were only initialized and never added to
+ * sysfs. Count them so that klp_free_patch_finish() can
+ * wait for all their releases before the patch module is
+ * unloaded. Without this, a delayed kobject release
+ * (CONFIG_DEBUG_KOBJECT_RELEASE) could access the freed
+ * module memory.
+ */
+ atomic_set(&patch->kobj_pending, 1); /* the patch kobject */
+ klp_for_each_object(patch, obj) {
+ atomic_inc(&patch->kobj_pending);
+ klp_for_each_func(obj, func)
+ atomic_inc(&patch->kobj_pending);
+ }
+ }
+
klp_free_objects(patch);
}
@@ -944,6 +980,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 +989,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)
@@ -966,6 +1004,8 @@ static void klp_init_patch_early(struct klp_patch *patch)
patch->forced = false;
INIT_WORK(&patch->free_work, klp_free_patch_work_fn);
init_completion(&patch->finish);
+ atomic_set(&patch->kobj_pending, 0);
+ patch->kobj_added = false;
klp_for_each_object_static(patch, obj) {
klp_init_object_early(patch, obj);
@@ -984,6 +1024,7 @@ static int klp_init_patch(struct klp_patch *patch)
ret = kobject_add(&patch->kobj, klp_root_kobj, "%s", patch->mod->name);
if (ret)
return ret;
+ patch->kobj_added = true;
if (patch->replace) {
ret = klp_add_nops(patch);
--
2.52.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] livepatch: Fix UAF of unregistered patch kobjects
2026-08-13 3:04 ` [PATCH 2/2] livepatch: Fix UAF of unregistered patch kobjects Yafang Shao
@ 2026-08-13 3:16 ` sashiko-bot
2026-08-13 3:58 ` Yafang Shao
0 siblings, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-08-13 3:16 UTC (permalink / raw)
To: joe.lawrence, jpoimboe, mbenes, jikos, pmladek, song, Yafang Shao
Cc: live-patching
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 <laoar.shao@gmail.com>
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/20260809094046.50ED31F000E9@smtp.kernel.org/
> 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.
> @@ -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.
[ ... ]
> @@ -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.
> + /*
> + * The kobjects were only initialized and never added to
> + * sysfs. Count them so that klp_free_patch_finish() can
> + * wait for all their releases before the patch module is
> + * unloaded. Without this, a delayed kobject release
> + * (CONFIG_DEBUG_KOBJECT_RELEASE) could access the freed
> + * module memory.
> + */
> + atomic_set(&patch->kobj_pending, 1); /* the patch kobject */
> + klp_for_each_object(patch, obj) {
> + atomic_inc(&patch->kobj_pending);
> + klp_for_each_func(obj, func)
> + atomic_inc(&patch->kobj_pending);
> + }
> + }
> +
> klp_free_objects(patch);
> }
[ ... ]
> @@ -987,6 +1027,7 @@ static int klp_init_patch(struct klp_patch *patch)
> ret = kobject_add(&patch->kobj, klp_root_kobj, "%s", patch->mod->name);
> if (ret)
> return ret;
> + patch->kobj_added = true;
>
> if (patch->replace) {
> ret = klp_add_nops(patch);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813030408.9761-1-laoar.shao@gmail.com?part=2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] livepatch: Fix UAF of unregistered patch kobjects
2026-08-13 3:16 ` sashiko-bot
@ 2026-08-13 3:58 ` Yafang Shao
0 siblings, 0 replies; 5+ messages in thread
From: Yafang Shao @ 2026-08-13 3:58 UTC (permalink / raw)
To: sashiko-reviews
Cc: joe.lawrence, jpoimboe, mbenes, jikos, pmladek, song,
live-patching
On Thu, Aug 13, 2026 at 11:16 AM <sashiko-bot@kernel.org> 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 <laoar.shao@gmail.com>
>
> 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/20260809094046.50ED31F000E9@smtp.kernel.org/
>
> > 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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-13 3:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 3:04 [PATCH 0/2] livepatch: Fix several bugs found during replace set implementation Yafang Shao
2026-08-13 3:04 ` [PATCH 1/2] livepatch: Fix wrong index in funcs cleanup error path Yafang Shao
2026-08-13 3:04 ` [PATCH 2/2] livepatch: Fix UAF of unregistered patch kobjects Yafang Shao
2026-08-13 3:16 ` sashiko-bot
2026-08-13 3:58 ` Yafang Shao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox