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

[Petr Mladek <pmladek@suse.com>]

On Fri 2024-05-03 14:14:34, Josh Poimboeuf wrote:
> On Sun, Apr 07, 2024 at 11:57:30AM +0800, Yafang Shao wrote:
> >   $ ls /sys/kernel/livepatch/
> >   livepatch_test_1                  <<<< livepatch_test_0 was replaced
> > 
> >   $ cat /sys/kernel/livepatch/livepatch_test_1/enabled
> >   1
> > 
> >   $ lsmod  | grep livepatch
> >   livepatch_test_1       16384  1
> >   livepatch_test_0       16384  0    <<<< leftover
> > 
> > Detecting which livepatch will be replaced by the new one from userspace is
> > not reliable

BTW: we handle this in rpm post install script. It removes all not longer
used livepatch modules before installing the newly installed one.


> > , necessitating the need for the operation to be performed
> > within the kernel itself. With this improvement, executing
> > `insmod livepatch-test_1.ko` will automatically remove the
> > livepatch-test_0.ko module.
> >
> > Following this change, the associated kernel module will be removed when
> > executing `echo 0 > /sys/kernel/livepatch/${livepath}/enabled`. Therefore,
> > adjustments need to be made to the selftests accordingly.
> 
> If the problem is that the user can't see which livepatch has been
> disabled, we should just fix that problem directly by leaving the
> disabled module in /sys/kernel/livepatch with an 'enabled' value of 0.
> 'enabled' could then be made read-only for replaced files.

I agree that it might remove the race. We must make sure that the
value is false only when the module can be removed. Also it would
require adding an API to remove the sysfs files from the module_exit
callback.

> That seems less disruptive to the user (and more consistent with the
> previous interface), and continues to leave the policy up to the user to
> decide if/when they want to remove the module.

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.

It would be a new behavior even for the module loader. But we could
see it as a version upgrade of a kernel module.

> It would also allow easily downgrading the replaced module in the future
> (once we have proper support for that).

Sigh, I still havn't found time to prepare v2 of the patchset
reworking the callbacks.

Best Regards,
Petr