Re: [Devel] [PATCH RK VZ9/VZ10] kpatch: pin current livepatch module

[no-reply@sashiko.virtuozzo.com]

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