Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1690164 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] livepatch: add (un)patch hooks Miroslav Benes <mbenes@suse.cz> - 2017-07-18 13:20 +0200
    Re: [PATCH] livepatch: add (un)patch hooks Josh Poimboeuf <jpoimboe@redhat.com> - 2017-07-19 04:10 +0200
      Re: [PATCH] livepatch: add (un)patch hooks Petr Mladek <pmladek@suse.com> - 2017-07-19 17:40 +0200
      Re: [PATCH] livepatch: add (un)patch hooks Miroslav Benes <mbenes@suse.cz> - 2017-07-19 21:20 +0200

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

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.
 
> > The patch-hook executes right before patching objects and the
> > unpatch-hook executes right after unpatching objects.
> > 
> > Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
> 
> Thanks for posting it.  We found this to be a useful feature in the
> past, not quite as useful as shadow data, but still good to have for
> certain cases.
> 
> Instead of "load hooks" I think it would be more accurate to call them
> "enable/disable hooks".  (Maybe "callbacks" would be better than
> "hooks"?  Not sure...)
> 
> Even better, we might want to be specific: "pre enable hooks" and "post
> disable hooks".  (Or "pre patch hooks" and "post unpatch hooks"?)
> Because we might eventually decide we need the corresponding "post
> enable hooks" and "pre disable hooks" as well.

And this is what I'm worried about. I think we don't want to have hooks 
sprinkled here and there in the code.

Thanks,
Miroslav

[toc] | [next] | [standalone]

#1690908

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.

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

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1691872

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@pathway.suse.cz
for more details.


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

Good point.

Best Regards,
Petr

[toc] | [prev] | [next] | [standalone]

#1692124

On Tue, 18 Jul 2017, 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.
> 
> - The module notifier can't stop the to-be-patched module from loading.

Ah, right. Thanks.

Joe, could you add both points to the changelog and the documentation, 
please?

Miroslav

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web