On Wed, May 8, 2024 at 3:03 PM Josh Poimboeuf [off-list ref] wrote:

On Wed, May 08, 2024 at 02:01:29PM +0800, Yafang Shao wrote:

quoted

On Wed, May 8, 2024 at 1:16 PM Josh Poimboeuf [off-list ref] wrote:

quoted

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.

When users disable the livepatch, the corresponding kernel module may
sometimes be removed, while other times it remains intact. This
inconsistency has the potential to confuse users.

quoted

quoted

quoted

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.

quoted

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 :-)

quoted

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

This scenario is typical for a regular kernel module, but I'm
uncertain if the same applies to a disabled livepatch. Is there a
mechanism in place to ensure that the corresponding kernel module is
always removed?

-- 
Regards
Yafang