Re: [PATCH 2/2] livepatch: Clean up klp_init_object_loaded() when fails

[Song Liu]

On Fri, Aug 28, 2026 at 5:53 AM Petr Mladek <pmladek@xxxxxxxx> wrote:
>
> When a module is loaded, klp_module_coming() iterates over patches and
> calls klp_init_object_loaded(). If initialization fails, it delegates
> cleanup to klp_cleanup_module_patches_limited().
>
> However, the cleanup loop skips the failing patch. Each function called
> in klp_init_object_loaded() is supposed to clean its own changes. This
> works except for the changes done by klp_init_object_loaded().
>
> The current code is a bit messy. The changes done by
> klp_init_object_loaded() should get cleared by klp_free_object_loaded().
> But this function also clears obj->mod which is set by
> klp_module_coming(). And relocations are cleared separately.
>
> Fix the situations by updating klp_free_object_loaded(). It should
> revert all and only changes made by klp_init_object_loaded().
> This requires some shuffling:
>
>  + Clear obj->mod explicitly in klp_cleanup_module_patches_limited()
>    and do not rely on klp_free_object_loaded().
>
>  + Clear relocations in klp_free_object_loaded(). Remove the explicit
>    call from klp_cleanup_module_patches_limited(). This requires
>    adding the @patch parameter.
>
> Finally, call klp_free_object_loaded() in the error path in
> klp_init_object_loaded().
>
> Reported-by: sashiko-bot@xxxxxxxxxx
> Closes: https://lore.kernel.org/r/20260823062313.1321B1F000E9@xxxxxxxxxxxxxxx
> Signed-off-by: Petr Mladek <pmladek@xxxxxxxx>

Acked-by: Song Liu <song@xxxxxxxxxx>

With one nitpick

> ---
>  kernel/livepatch/core.c | 30 ++++++++++++++++++++----------
>  1 file changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index cdb25949f73b..1e59a3cc0895 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -725,18 +725,20 @@ static void __klp_free_funcs(struct klp_object *obj, bool nops_only)
>  }
>
>  /* Clean up when a patched object is unloaded */
> -static void klp_free_object_loaded(struct klp_object *obj)
> +static void klp_free_object_loaded(struct klp_patch *patch,
> +                                  struct klp_object *obj)

nit: Do we still need to fit every line in 80 characters? checkpatch.pl
only enforce 100 characters these days.

Thanks
Song

>  {
>         struct klp_func *func;

[...]