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

[Petr Mladek <pmladek@suse.com>]

On Mon 2024-05-06 19:35:22, Josh Poimboeuf wrote:
> On Mon, May 06, 2024 at 01:32:19PM +0200, Petr Mladek wrote:
> > Also it would require adding an API to remove the sysfs files from the
> > module_exit callback.
> 
> Could the sysfs removal be triggered from klp_module_going() or a module
> notifier?

Great idea!

> > I do not see any reasonable reason to keep the replaced livepatch
> > module loaded. It is an unusable piece of code. IMHO, it would be
> > really convenient if the kernel removed it.
> 
> User space needs to be polling for the transition to complete so it can
> reverse the patch if it stalls.  Otherwise the patch could stall forever
> and go unnoticed.

Triggering the revert and removing the module are two asynchronous
operations. User space might just trigger the revert when it detect
the stall and assume that the module will get removed.

Stalled revert is end of the game anyway.

> Can't user space just unload the replaced module after it detects the
> completed transition?
> 
> I'm not sure I see the benefit in complicating the kernel and possibly
> introducing bugs, when unloading the module from user space seems to be
> a perfectly valid option.
>
> Also, an error returned by delete_module() to the kernel would be
> ignored and the module might remain in memory forever without being
> noticed.

The same error would happen even when it was called via the syscall.
It is the same code.

Well, user space might want to know when the module was removed.
For example, it could not load the same livepatch module until
it was removed. Explicit rmmod would be useful in this case.

I agree that adding the auto deleting might create new problems
and probably is not worth it.

Best Regards,
Petr