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

[Petr Mladek]

On Fri 2026-08-28 10:44:38, Song Liu wrote:
> 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

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

I do not have strong opinion about it. My editor highlights characters
which are over the 80 lines limit so I automatically fix it. I haven't
found a courage to change it yet. And my eyes are getting worse over
the years so I use big fonts. 80 characters per line look good
on my monitor ;-)

Best Regards,
Petr