Re: [PATCH] livepatch: add (un)patch hooks

From: Petr Mladek
Date: Wed Jul 19 2017 - 11:35:20 EST


On Tue 2017-07-18 21:08:57, Josh Poimboeuf wrote:
> On Tue, Jul 18, 2017 at 01:15:16PM +0200, Miroslav Benes wrote:
> > On Thu, 13 Jul 2017, Josh Poimboeuf wrote:
> >
> > > On Wed, Jul 12, 2017 at 10:10:00AM -0400, Joe Lawrence wrote:
> > > > When the livepatch core executes klp_(un)patch_object, call out to a
> > > > livepatch-module specified array of callback hooks. These hooks provide
> > > > a notification mechanism for livepatch modules when klp_objects are
> > > > (un)patching. This may be most interesting when another kernel module
> > > > is a klp_object target and the livepatch module needs to execute code
> > > > after the target is loaded, but before its module_init code is run.
> > >
> > > And it's also useful for vmlinux. Patch module load/unload is separate
> > > from enable/disable, so the module init/exit functions can't be used for
> > > patch-specific changes (e.g., global data changes).
> >
> > I admit that I don't understand this, which is probably the reason for my
> > question. Why do we need it when we have module notifiers and module
> > init/exit functions in the kernel? Petr described different possible
> > scenarios and they can be solved either in init/exit function of a patch
> > module or in a module notifier which the patch module can register.
> >
> > If there is a difference, it should be mentioned in the documentation and
> > in the changelog.
>
> Some differences:
>
> - The patch module init/exit code doesn't run when disabling and
> re-enabling a patch.

True. Well, I would still like to see some real life examples
where this can be used.

My problem with the callbacks is that the same code runs in too
many situations and each situation has different constrains.
IMHO, it is much more complicated to say what is safe and what
is not. See
https://lkml.kernel.org/r/20170717155144.GF32632@xxxxxxxxxxxxxxx
for more details.


> - The module notifier can't stop the to-be-patched module from loading.

Good point.

Best Regards,
Petr