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

Yafang Shao
2024-05-09T11:57:55Z

On Thu, May 9, 2024 at 6:16 PM Petr Mladek <pmladek@suse.com> wrote:
>
> On Thu 2024-05-09 13:53:17, Yafang Shao wrote:
> > On Thu, May 9, 2024 at 1:20 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > >
> > > On Thu, May 09, 2024 at 10:17:43AM +0800, Yafang Shao wrote:
> > > > On Wed, May 8, 2024 at 3:03 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > > > >
> > > > > 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@kernel.org> 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.
> > > >
> > > > 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.
> > >
> > > I'm afraid I don't understand. Can you give an example scenario?
> > >
> >
> > As previously mentioned, this scenario may occur if user-space tools
> > remove all pertinent kernel modules from /sys/livepatch/* while a user
> > attempts to load a new atomic-replace livepatch.
> >
> > For instance:
> >
> > User-A User-B
> >
> > echo 0 > /sys/livepatch/A/enable insmod atomic-replace-patch.ko
> >
> > >From User-A's viewpoint, the A.ko module might sometimes be removed,
> > while at other times it remains intact. The reason is that User-B
> > removed a module that he shouldn't remove.
>
> Why would User-A want to keep the module, please? The livepatches
> could not longer be re-enabled since the commit 958ef1e39d24d6
> ("livepatch: Simplify API by removing registration step") which
> was added in v5.1-rc1.
>
> The only problem might be that User-A can't remove the module
> because it has already been removed by User-B or vice versa.
> Is this really a problem?
>
> Have you seen the problem in practice or is it just theoretical?

It is just theoretical.

>
> Is anyone really combining livepatches with and without atomic
> replace?

I don't know.

Previously, we utilized live patches without atomic-replace
functionality. However, we've now transitioned all patches to support
atomic-replace. It might be beneficial to introduce a
/sys/kernel/livepatch/XXX/replace entry to indicate whether a patch
supports atomic-replace or not

--
Regards
Yafang