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

Groups > linux.kernel > #1722572

Re: [PATCH v4 1/3] livepatch: add (un)patch callbacks

Show all headers | View raw

On Fri, Aug 25, 2017 at 03:10:00PM -0400, Joe Lawrence wrote:
> +Test 6
> +------
> +
> +Test a scenario where a vmlinux pre-patch callback returns a non-zero
> +status (ie, failure):
> +
> +- load target module
> +- load livepatch -ENODEV
> +- unload target module
> +
> +First load a target module:
> +
> +  % insmod samples/livepatch/livepatch-callbacks-mod.ko
> +  [   80.740520] livepatch_callbacks_mod: livepatch_callbacks_mod_init
> +
> +Load the livepatch module, setting its 'pre_patch_ret' value to -19
> +(-ENODEV).  When its vmlinux pre-patch callback executed, this status
> +code will propagate back to the module-loading subsystem.  The result is
> +that the insmod command refuses to load the livepatch module:
> +
> +  % insmod samples/livepatch/livepatch-callbacks-demo.ko pre_patch_ret=-19
> +  [   82.747326] livepatch: enabling patch 'livepatch_callbacks_demo'
> +  [   82.747743] livepatch: 'livepatch_callbacks_demo': initializing unpatching transition
> +  [   82.747767] livepatch_callbacks_demo: pre_patch_callback: vmlinux
> +  [   82.748237] livepatch: pre-patch callback failed for object 'vmlinux'
> +  [   82.748637] livepatch: failed to enable patch 'livepatch_callbacks_demo'
> +  [   82.749059] livepatch: 'livepatch_callbacks_demo': canceling transition, unpatching
> +  [   82.749060] livepatch: 'livepatch_callbacks_demo': completing unpatching transition
> +  [   82.749177] livepatch_callbacks_demo: post_unpatch_callback: livepatch_callbacks_mod -> [MODULE_STATE_LIVE] Normal state
> +  [   82.749868] livepatch: 'livepatch_callbacks_demo': unpatching complete
> +  [   82.765809] insmod: ERROR: could not insert module samples/livepatch/livepatch-callbacks-demo.ko: No such device
> +
> +  % rmmod samples/livepatch/livepatch-callbacks-mod.ko
> +  [   84.774238] livepatch_callbacks_mod: livepatch_callbacks_mod_exit

First off, this documentation is very nice because it clarifies all the
callback scenarios and edge cases.

The above situation still seems a little odd to me.  If I understand
correctly, the target module was never patched, and its pre_patch
callback was never called.  But its post_unpatch callback *was* called.
That doesn't seem right.

Maybe we should change the condition a little bit.  Currently it's:

  No post-patch, pre-unpatch, or post-unpatch callbacks will be executed
  for a given klp_object if its pre-patch callback returned non-zero
  status.

I think that might have been my idea, but seeing the above case makes it
clear that it's not quite right.  Maybe it should instead be:

  No post-patch, pre-unpatch, or post-unpatch callbacks will be executed
  for a given klp_object if the object failed to patch, due to a failed
  pre_patch callback or for any other reason.

  If the object did successfully patch, but the patch transition never
  started for some reason (e.g., if another object failed to patch),
  only the post-unpatch callback will be called.

So then, instead of tracking whether the pre-patch callback succeeded,
we just need to track whether the object was patched (which we already
do, with obj->patched).

What do you think?

-- 
Josh

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v4 0/3] livepatch callbacks Joe Lawrence <joe.lawrence@redhat.com> - 2017-08-25 21:20 +0200
  [PATCH v4 2/3] livepatch: move transition "complete" notice into klp_complete_transition() Joe Lawrence <joe.lawrence@redhat.com> - 2017-08-25 21:20 +0200
    Re: [PATCH v4 2/3] livepatch: move transition "complete" notice into  klp_complete_transition() Josh Poimboeuf <jpoimboe@redhat.com> - 2017-08-29 18:00 +0200
  [PATCH v4 3/3] livepatch: add transition notices Joe Lawrence <joe.lawrence@redhat.com> - 2017-08-25 21:20 +0200
    Re: [PATCH v4 3/3] livepatch: add transition notices Josh Poimboeuf <jpoimboe@redhat.com> - 2017-08-29 18:00 +0200
  Re: [PATCH v4 1/3] livepatch: add (un)patch callbacks Josh Poimboeuf <jpoimboe@redhat.com> - 2017-08-29 17:50 +0200
    Re: [PATCH v4 1/3] livepatch: add (un)patch callbacks Joe Lawrence <joe.lawrence@redhat.com> - 2017-08-29 21:30 +0200
      Re: [PATCH v4 1/3] livepatch: add (un)patch callbacks Josh Poimboeuf <jpoimboe@redhat.com> - 2017-08-29 22:00 +0200
        Re: [PATCH v4 1/3] livepatch: add (un)patch callbacks Joe Lawrence <joe.lawrence@redhat.com> - 2017-08-30 15:30 +0200
          Re: [PATCH v4 1/3] livepatch: add (un)patch callbacks Josh Poimboeuf <jpoimboe@redhat.com> - 2017-08-30 16:30 +0200
  Re: [PATCH v4 1/3] livepatch: add (un)patch callbacks Josh Poimboeuf <jpoimboe@redhat.com> - 2017-08-30 16:50 +0200

csiph-web