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

Yafang Shao
2024-05-07T14:04:36Z

On Tue, May 7, 2024 at 10:35 AM Josh Poimboeuf <jpoimboe@kernel.org> 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?
>
> > 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.
>
> Can't user space just unload the replaced module after it detects the
> completed transition?

Are you referring to polling the
"/sys/kernel/livepatch/XXX/transition"? The challenge lies in the
uncertainty regarding which livepatches will be replaced and how many.
Even if we can poll the transition status, there's no guarantee that a
livepatch will be replaced by this operation.

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

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.

--
Regards
Yafang