Re: [PATCH v6 0/3] livepatch callbacks

[Jiri Kosina]

On Fri, 13 Oct 2017, Joe Lawrence wrote:

> Another turn of the livepatch callback crank.  This version cleans up
> some minor issues found in v5 and was rebased on top of Jiri's branches
> listed below.
> 
> Unfortunately, I didn't have time to test out Petr's suggestion to move
> the pre-patch callback inside klp_patch_object() and the post-unpatch
> callback into klp_unpatch_object()... let me know if this is a
> deal-breaker (I think it would be possible to revisit this later if
> need be.)
> 
> Thanks for all the feedback and suggestions thus far!

Applied to for-4.15/callbacks branch, thanks a lot.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v6 0/3] livepatch callbacks

[Jiri Kosina]

On Fri, 13 Oct 2017, Joe Lawrence wrote:

> Another turn of the livepatch callback crank.  This version cleans up
> some minor issues found in v5 and was rebased on top of Jiri's branches
> listed below.
> 
> Unfortunately, I didn't have time to test out Petr's suggestion to move
> the pre-patch callback inside klp_patch_object() and the post-unpatch
> callback into klp_unpatch_object()... let me know if this is a
> deal-breaker (I think it would be possible to revisit this later if
> need be.)
> 
> Thanks for all the feedback and suggestions thus far!

Applied to for-4.15/callbacks branch, thanks a lot.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v6 0/3] livepatch callbacks

[Josh Poimboeuf]

On Fri, Oct 13, 2017 at 03:08:40PM -0400, Joe Lawrence wrote:
> Another turn of the livepatch callback crank.  This version cleans up
> some minor issues found in v5 and was rebased on top of Jiri's branches
> listed below.
> 
> Unfortunately, I didn't have time to test out Petr's suggestion to move
> the pre-patch callback inside klp_patch_object() and the post-unpatch
> callback into klp_unpatch_object()... let me know if this is a
> deal-breaker (I think it would be possible to revisit this later if
> need be.)
> 
> Thanks for all the feedback and suggestions thus far!

Looks perfect!  I'm hoping we can get this in for the next merge window.

Acked-by: Josh Poimboeuf 

-- 
Josh

Re: [PATCH v6 0/3] livepatch callbacks

[Josh Poimboeuf]

On Fri, Oct 13, 2017 at 03:08:40PM -0400, Joe Lawrence wrote:
> Another turn of the livepatch callback crank.  This version cleans up
> some minor issues found in v5 and was rebased on top of Jiri's branches
> listed below.
> 
> Unfortunately, I didn't have time to test out Petr's suggestion to move
> the pre-patch callback inside klp_patch_object() and the post-unpatch
> callback into klp_unpatch_object()... let me know if this is a
> deal-breaker (I think it would be possible to revisit this later if
> need be.)
> 
> Thanks for all the feedback and suggestions thus far!

Looks perfect!  I'm hoping we can get this in for the next merge window.

Acked-by: Josh Poimboeuf 

-- 
Josh

[PATCH v6 0/3] livepatch callbacks

[Joe Lawrence]

Another turn of the livepatch callback crank.  This version cleans up
some minor issues found in v5 and was rebased on top of Jiri's branches
listed below.

Unfortunately, I didn't have time to test out Petr's suggestion to move
the pre-patch callback inside klp_patch_object() and the post-unpatch
callback into klp_unpatch_object()... let me know if this is a
deal-breaker (I think it would be possible to revisit this later if
need be.)

Thanks for all the feedback and suggestions thus far!

-- Joe


v6:

- Rebased on-top of jiri_livepatching/
- for-4.14/upstream-fixes
- for-4.15/shadow-variables

Rebasing changes:
  - kernel/livepatch/core.c: code moved from klp_module_going() to
klp_cleanup_module_patches_limited()
  - samples/livepatch/Makefile: patch context changes due to shadow
variable samples

- Documentation
  - s/pare/pair
  - add note about reversed transitions and post-patch/pre-unpatch
callbacks
  - add Josh's extended text about which callbacks will run

- Debug messages
  - Fix pr_debug at beginning of klp_init_transition() to display the
intended target state
  - Expand "canceling patching transition" pr_debug message

- Misc
  - s/callbacks_enabled/post_unpatch_enabled and moved to struct
klp_callbacks per Miroslav's suggestion
  - Added Acked-by and Reviewed-by tags to patches 1 and 2


v5:

- Move code comments from kernel/livepatch/core.h to
  include/linux/livepatch.h's struct klp_callbacks definition

- Change int klp_object.prepatch_callback_status to a bool
  klp_object.callbacks_enabled

  - fixes a hole where returned status=0 wasn't distinguished from
"never-ran" status=0

- Removed a few extraneous checks in the execute callback routines

- In the module-coming case, be sure to execute the post-unpatch
  callback in the event that patching fails

- Moved my Signed-off-by line to the bottom in patches 2 and 3


v4:

- Move callback helpers into core.h

- Move klp_pre_patch_callback() and klp_pre_unpatch_callback()
  invocations into __klp_enable_patch() and __klp_disable_patch()

  - klp_patch_object() and klp_unpatch_objects()
- Do not run pre-unpatch callbacks from here

- Add a pre_patch_status member to klp_object so when a pre-patch
  callback fails, the helpers can skip any post-patch, pre-unpatch,
  post-unpatch callbacks

- klp_module_coming() and klp_module_going()
  - Do not run post-patch or pre-unpatch callbacks for current
klp_transition_patch

- Documentation
  - Add various test cases and provide commentary

- Samples
  - Create two target modules: a simple one and another that invokes a
worker function that sleeps for a long time

- Added two follow-up patches:

  - livepatch: move transition "complete" notice into
klp_complete_transition() - this pushes the "patching complete" message
after the post-patch callbacks

  - livepatch: add transition notices - these were helpful during
debugging of the callback patch.  The transaction annotations were
also used in the Documentation file tese cases to illustrate the
order of operations.


v3:

- livepatch.h
  - drop obj->patched checks from pre/post-(un)patch funcs,
add preceding comment and note about obj->patched assumptions
  - move core.c :: klp_is_module() to here

- klp_complete_transition()
  - fix "else if (klp_target_state == KLP_UNPATCHED)" case
  - combine conditional syntax when avoiding module_put for immediate
patches
  - add check for klp_is_object_loaded to avoid callbacks for any
unloaded modules (necessary after removing obj->patched checks in
livepatch.h)

- Documentation
  - added Josh's use-cases blurb in intro
  - s/Callbacks are only executed/A callbacks is only executed/

- livepatch-callbacks-demo.c
  - whitespace cleanup


v2:
- s/hooks/callbacks/g
- implemented pre-(un)patch and post-(un)patch callbacks
  - pre-patch and pre-unpatch callbacks run from callers of
klp_patch_object() and klp_unpatch_object()
  - post-patch and post-unpatch callbacks run from
klp_complete_transition() and klp_module_coming/going()
- reduce callbacks from a list to a single per-klp_object instance
- revamp the sample callback demo
- revamp documentation

Joe Lawrence (3):
  livepatch: add (un)patch callbacks
  livepatch: move transition "complete" notice into
klp_complete_transition()
  livepatch: add transition notices

 Documentation/livepatch/callbacks.txt   | 605 
 include/linux/livepatch.h   |  26 +
 kernel/livepatch/core.c |  51 +-
 kernel/livepatch/core.h |  38 ++
 kernel/livepatch/patch.c|   1 +
 kernel/livepatch/transition.c   |  45 +-
 samples/livepatch/Makefile  |   3 +
 samples/livepatch/livepatch-callbacks-busymod.c |  72 +++
 samples/livepatch/livepatch-callbacks-demo.c| 234 +
 samples/livepatch/livepatch-callbacks-mod.c  

[PATCH v6 0/3] livepatch callbacks

[Joe Lawrence]

Another turn of the livepatch callback crank.  This version cleans up
some minor issues found in v5 and was rebased on top of Jiri's branches
listed below.

Unfortunately, I didn't have time to test out Petr's suggestion to move
the pre-patch callback inside klp_patch_object() and the post-unpatch
callback into klp_unpatch_object()... let me know if this is a
deal-breaker (I think it would be possible to revisit this later if
need be.)

Thanks for all the feedback and suggestions thus far!

-- Joe


v6:

- Rebased on-top of jiri_livepatching/
- for-4.14/upstream-fixes
- for-4.15/shadow-variables

Rebasing changes:
  - kernel/livepatch/core.c: code moved from klp_module_going() to
klp_cleanup_module_patches_limited()
  - samples/livepatch/Makefile: patch context changes due to shadow
variable samples

- Documentation
  - s/pare/pair
  - add note about reversed transitions and post-patch/pre-unpatch
callbacks
  - add Josh's extended text about which callbacks will run

- Debug messages
  - Fix pr_debug at beginning of klp_init_transition() to display the
intended target state
  - Expand "canceling patching transition" pr_debug message

- Misc
  - s/callbacks_enabled/post_unpatch_enabled and moved to struct
klp_callbacks per Miroslav's suggestion
  - Added Acked-by and Reviewed-by tags to patches 1 and 2


v5:

- Move code comments from kernel/livepatch/core.h to
  include/linux/livepatch.h's struct klp_callbacks definition

- Change int klp_object.prepatch_callback_status to a bool
  klp_object.callbacks_enabled

  - fixes a hole where returned status=0 wasn't distinguished from
"never-ran" status=0

- Removed a few extraneous checks in the execute callback routines

- In the module-coming case, be sure to execute the post-unpatch
  callback in the event that patching fails

- Moved my Signed-off-by line to the bottom in patches 2 and 3


v4:

- Move callback helpers into core.h

- Move klp_pre_patch_callback() and klp_pre_unpatch_callback()
  invocations into __klp_enable_patch() and __klp_disable_patch()

  - klp_patch_object() and klp_unpatch_objects()
- Do not run pre-unpatch callbacks from here

- Add a pre_patch_status member to klp_object so when a pre-patch
  callback fails, the helpers can skip any post-patch, pre-unpatch,
  post-unpatch callbacks

- klp_module_coming() and klp_module_going()
  - Do not run post-patch or pre-unpatch callbacks for current
klp_transition_patch

- Documentation
  - Add various test cases and provide commentary

- Samples
  - Create two target modules: a simple one and another that invokes a
worker function that sleeps for a long time

- Added two follow-up patches:

  - livepatch: move transition "complete" notice into
klp_complete_transition() - this pushes the "patching complete" message
after the post-patch callbacks

  - livepatch: add transition notices - these were helpful during
debugging of the callback patch.  The transaction annotations were
also used in the Documentation file tese cases to illustrate the
order of operations.


v3:

- livepatch.h
  - drop obj->patched checks from pre/post-(un)patch funcs,
add preceding comment and note about obj->patched assumptions
  - move core.c :: klp_is_module() to here

- klp_complete_transition()
  - fix "else if (klp_target_state == KLP_UNPATCHED)" case
  - combine conditional syntax when avoiding module_put for immediate
patches
  - add check for klp_is_object_loaded to avoid callbacks for any
unloaded modules (necessary after removing obj->patched checks in
livepatch.h)

- Documentation
  - added Josh's use-cases blurb in intro
  - s/Callbacks are only executed/A callbacks is only executed/

- livepatch-callbacks-demo.c
  - whitespace cleanup


v2:
- s/hooks/callbacks/g
- implemented pre-(un)patch and post-(un)patch callbacks
  - pre-patch and pre-unpatch callbacks run from callers of
klp_patch_object() and klp_unpatch_object()
  - post-patch and post-unpatch callbacks run from
klp_complete_transition() and klp_module_coming/going()
- reduce callbacks from a list to a single per-klp_object instance
- revamp the sample callback demo
- revamp documentation

Joe Lawrence (3):
  livepatch: add (un)patch callbacks
  livepatch: move transition "complete" notice into
klp_complete_transition()
  livepatch: add transition notices

 Documentation/livepatch/callbacks.txt   | 605 
 include/linux/livepatch.h   |  26 +
 kernel/livepatch/core.c |  51 +-
 kernel/livepatch/core.h |  38 ++
 kernel/livepatch/patch.c|   1 +
 kernel/livepatch/transition.c   |  45 +-
 samples/livepatch/Makefile  |   3 +
 samples/livepatch/livepatch-callbacks-busymod.c |  72 +++
 samples/livepatch/livepatch-callbacks-demo.c| 234 +
 samples/livepatch/livepatch-callbacks-mod.c