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@xxxxxxxxxx> 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
