Re: [PATCH v2 2/2] livepatch: Delete the associated module of disabled livepatch

[Josh Poimboeuf <jpoimboe@kernel.org>]

On Wed, May 08, 2024 at 02:01:29PM +0800, Yafang Shao wrote:
> On Wed, May 8, 2024 at 1:16 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > If klp_patch.replace is set on the new patch then it will replace all
> > previous patches.
> 
> A scenario exists wherein a user could simultaneously disable a loaded
> livepatch, potentially resulting in it not being replaced by the new
> patch. While theoretical, this possibility is not entirely
> implausible.

Why does it matter whether it was replaced, or was disabled beforehand?
Either way the end result is the same.

> > > As Petr pointed out, we can enhance the functionality by checking the
> > > return value and providing informative error messages. This aligns
> > > with the user experience when deleting a module; if deletion fails,
> > > users have the option to try again. Similarly, if error messages are
> > > displayed, users can manually remove the module if needed.
> >
> > Calling delete_module() from the kernel means there's no syscall with
> > which to return an error back to the user.
> 
> pr_error() can calso tell the user the error, right ?

The dmesg buffer isn't a reliable way to communicate errors to a user
space process.

> If we must return an error to the user, probably we can use
> klp_free_replaced_patches_sync() instead of
> klp_free_replaced_patches_async().

It's async for a reason :-)

> Under what conditions might a kernel module of a disabled livepatch be
> unable to be removed?

For example:

  - Some code grabbed a reference to it, or some module has a dependency
    on it

  - It has an init function but not an exit function

  - The module has already been removed due to some race

  - Some other unforeseen bug or race in the module exit path

-- 
Josh