* Re: [Devel] [PATCH RK VZ9/VZ10] kpatch: pin current livepatch module
2026-11-03 20:24 [Devel] [PATCH RK VZ9/VZ10] kpatch: pin current livepatch module Vladimir Riabchun
@ 2026-11-03 20:29 ` no-reply
0 siblings, 0 replies; 2+ messages in thread
From: no-reply @ 2026-11-03 20:29 UTC (permalink / raw)
To: Vladimir Riabchun, devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] time: livepatch-only code in kernel/time/namespace.c breaks build without LIVEPATCH
--
commit 52de3044b09394f30e256815b7a2bb469fb6abec
Author: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
kpatch: pin current livepatch module
This patch appends a livepatch post-patch callback, kpatch_pin_self(), to
kernel/time/namespace.c. The callback takes an extra module reference on
the livepatch module so that it cannot be unloaded.
> diff --git a/kernel/time/namespace.c b/kernel/time/namespace.c
> index cf95f9e0ad94..b6f589a2dbff 100644
> --- a/kernel/time/namespace.c
> +++ b/kernel/time/namespace.c
[ ... ]
> @@ -374,3 +374,14 @@ struct time_namespace init_time_ns = {
> .ns.ops = &timens_operations,
> .frozen_offsets = true,
> };
> +
> +#include <linux/livepatch_helpers.h>
> +
> +static void kpatch_pin_self(struct klp_object *obj)
> +{
> + struct module *mod = container_of(obj->kobj.parent, struct klp_patch, kobj)->mod;
[Severity: Low]
Can this build when CONFIG_LIVEPATCH is disabled?
kernel/time/namespace.c is built whenever CONFIG_TIME_NS=y, but
kpatch_pin_self() and the include of livepatch_helpers.h are added with no
CONFIG_LIVEPATCH guard.
In include/linux/livepatch.h, struct klp_patch and struct klp_object are
defined only inside the conditional block:
#if IS_ENABLED(CONFIG_LIVEPATCH)
The #else branch provides only stubs. livepatch_external.h only
forward-declares struct klp_object.
With CONFIG_LIVEPATCH=n, the following expressions would operate on
incomplete or undefined types:
obj->kobj.parent
container_of(..., struct klp_patch, kobj)->mod
KLP_POST_PATCH_CALLBACK() also uses klp_post_patch_t, which sits in the
same conditional. Would this make kernel/time/namespace.c fail to compile
for CONFIG_TIME_NS=y and CONFIG_LIVEPATCH=n?
Even with CONFIG_LIVEPATCH=y, a plain vmlinux build has KLP_OBJNAME set to
vmlinux. The callback pointer then lands in .discard.klp_callback_ptrs, and
kpatch_pin_self() is dead code.
This looks like it is meant to be a livepatch source delta. As written,
though, the commit has no CONFIG_LIVEPATCH guard.
This isn't a bug, but the #include of linux/livepatch_helpers.h in the
middle of the file, after the init_time_ns definition, is unusual. Could it
move to the top with the other includes?
> +
> + __module_get(mod);
> + pr_warn("Livepatch module %s is pinned and cannot be unloaded\n", module_name(mod));
> +}
> +KLP_POST_PATCH_CALLBACK(kpatch_pin_self);
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261103202407.738340-1-vladimir.riabchun@virtuozzo.com?part=1
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 2+ messages in thread