mbox series

[v2,00/12] livepatch: new features and fixes

Message ID 20190827084624.116917-1-wipawel@amazon.de (mailing list archive)
Headers show
Series livepatch: new features and fixes | expand

Message

Wieczorkiewicz, Pawel Aug. 27, 2019, 8:46 a.m. UTC
This series introduces new features to the livepatch functionality as
briefly discussed during Xen Developer Summit 2019: [a] and [b].
It also provides a few fixes and some small improvements.

Main changes in v2:
- added new features to livepatch documentation
- added livepatch tests
- enabled Arm support for [5]
- make .modinfo optional for [11]
- fixed typos

FEATURES:

1. independent modules (patches: [1], [2])

  * livepatch-build-tools repo dependency [A]

  Livepatch enforces the following buildid-based dependency chain
  between hotpatch modules:
    1) first module depends on given hypervisor buildid
    2) every consecutive module depends on previous module's buildid
  This way proper hotpatch stack order is maintained and enforced.
  While it is important for production hotpatches it limits agility and
  blocks usage of testing or debug hotpatches. These kinds of hotpatch
  modules are typically expected to be loaded at any time irrespective
  of current state of the modules stack.

  [A] livepatch-build: Embed hypervisor build id into every hotpatch

2. pre- and post- apply|revert actions hooks (patches: [3], [4])

  * livepatch-build-tools repo dependency [B]

  This is an implementation of 4 new livepatch module vetoing hooks,
  that can be optionally supplied along with modules.
  Hooks that currently exists in the livepatch mechanism aren't agile
  enough and have various limitations:
  * run only from within a quiescing zone
  * cannot conditionally prevent applying or reverting
  * do not have access to the module context
  To address these limitations the following has been implemented:
  1) pre-apply hook
  2) post-apply hook
  3) pre-revert hook
  4) post-revert hook

  [B] create-diff-object: Handle extra pre-|post- hooks

3. apply|revert actions replacement hooks (patches: [5], [6], [7])

  * livepatch-build-tools repo dependency: [C], [D], [E]

  To increase hotpatching system's agility and provide more flexiable
  long-term hotpatch solution, allow to overwrite the default apply
  and revert action functions with hook-like supplied alternatives.
  The alternative functions are optional and the default functions are
  used by default.

  [C] create-diff-object: Do not create empty .livepatch.funcs section
  [D] create-diff-object: Handle optional apply|revert hooks
  [E] create-diff-object: Add support for applied/reverted marker

4. inline asm hotpatching expectations (patches: [8])

  * livepatch-build-tools repo dependency: [F]

  Expectations are designed as optional feature, since the main use of
  them is planned for inline asm hotpatching.
  The payload structure is modified as each expectation structure is
  part of the livepatch_func structure and hence extends the payload.
  The payload version is bumped to 3 with this change to highlight the
  ABI modification and enforce proper support.
  The expectation is manually enabled during inline asm module
  construction. If enabled, expectation ensures that the expected
  content of memory is to be found at a given patching (old_addr)
  location.

  [F] create-diff-object: Add support for expectations

5. runtime hotpatch metadata support (patches: [9], [10], [11])

  Having detailed hotpatch metadata helps to properly identify module's
  origin and version. It also allows to keep track of the history of
  hotpatch loads in the system (at least within dmesg buffer size
  limits).
  Extend the livepatch list operation to fetch also payloads' metadata.
  This is achieved by extending the sysctl list interface with 2 extra
  guest handles:
  * metadata     - an array of arbitrary size strings
  * metadata_len - an array of metadata strings' lengths (uin32_t each)
  To unify and simplify the interface, handle the modules' name strings
  of arbitrary size by copying them in adhering chunks to the userland.

6. python bindings for livepatch operations (patches: [12])

  Extend the XC python bindings library to support all common livepatch
  operations and actions:
  - status (pyxc_livepatch_status):
  - action (pyxc_livepatch_action):
  - upload (pyxc_livepatch_upload):
  - list (pyxc_livepatch_list):

[a] https://wiki.xenproject.org/wiki/Design_Sessions_2019#LivePatch_improvements_and_features
[b] https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg00846.html

Merged in v1:
  python: Add XC binding for Xen build ID
  livepatch: always print XENLOG_ERR information

Pawel Wieczorkiewicz (12):
  [1] livepatch: Always check hypervisor build ID upon hotpatch upload
  [2] livepatch: Allow to override inter-modules buildid dependency
  [3] livepatch: Export payload structure via livepatch_payload.h
  [4] livepatch: Implement pre-|post- apply|revert hooks
  [5] livepatch: Add support for apply|revert action replacement hooks
  [6] livepatch: Do not enforce ELF_LIVEPATCH_FUNC section presence
  [7] livepatch: Add per-function applied/reverted state tracking marker
  [8] livepatch: Add support for inline asm hotpatching expectations
  [9] livepatch: Add support for modules .modinfo section metadata
  [10] livepatch: Handle arbitrary size names with the list operation
  [11] livepatch: Add metadata runtime retrieval mechanism
  [12] livepatch: Add python bindings for livepatch operations

 .gitignore                                     |   6 +-
 docs/misc/livepatch.pandoc                     | 231 ++++++++-
 tools/libxc/include/xenctrl.h                  |  68 ++-
 tools/libxc/xc_misc.c                          | 162 +++++--
 tools/misc/xen-livepatch.c                     | 258 +++++++---
 tools/python/xen/lowlevel/xc/xc.c              | 273 +++++++++++
 xen/arch/arm/arm32/livepatch.c                 |  12 +-
 xen/arch/arm/arm64/livepatch.c                 |  12 +-
 xen/arch/arm/livepatch.c                       |  10 +-
 xen/arch/x86/livepatch.c                       |  22 +-
 xen/common/livepatch.c                         | 647 +++++++++++++++++++++----
 xen/include/public/sysctl.h                    |  61 ++-
 xen/include/xen/livepatch.h                    |  42 +-
 xen/include/xen/livepatch_payload.h            |  83 ++++
 xen/test/livepatch/Makefile                    | 113 ++++-
 xen/test/livepatch/xen_action_hooks.c          | 102 ++++
 xen/test/livepatch/xen_action_hooks_marker.c   | 112 +++++
 xen/test/livepatch/xen_action_hooks_noapply.c  | 136 ++++++
 xen/test/livepatch/xen_action_hooks_nofunc.c   |  86 ++++
 xen/test/livepatch/xen_action_hooks_norevert.c | 143 ++++++
 xen/test/livepatch/xen_expectations.c          |  41 ++
 xen/test/livepatch/xen_expectations_fail.c     |  42 ++
 xen/test/livepatch/xen_prepost_hooks.c         | 122 +++++
 xen/test/livepatch/xen_prepost_hooks_fail.c    |  75 +++
 24 files changed, 2579 insertions(+), 280 deletions(-)
 create mode 100644 xen/test/livepatch/xen_action_hooks.c
 create mode 100644 xen/test/livepatch/xen_action_hooks_marker.c
 create mode 100644 xen/test/livepatch/xen_action_hooks_noapply.c
 create mode 100644 xen/test/livepatch/xen_action_hooks_nofunc.c
 create mode 100644 xen/test/livepatch/xen_action_hooks_norevert.c
 create mode 100644 xen/test/livepatch/xen_expectations.c
 create mode 100644 xen/test/livepatch/xen_expectations_fail.c
 create mode 100644 xen/test/livepatch/xen_prepost_hooks.c
 create mode 100644 xen/test/livepatch/xen_prepost_hooks_fail.c

Comments

Konrad Rzeszutek Wilk Aug. 29, 2019, 7:23 p.m. UTC | #1
> Pawel Wieczorkiewicz (12):
>   [1] livepatch: Always check hypervisor build ID upon hotpatch upload
>   [2] livepatch: Allow to override inter-modules buildid dependency
>   [3] livepatch: Export payload structure via livepatch_payload.h
>   [4] livepatch: Implement pre-|post- apply|revert hooks
>   [5] livepatch: Add support for apply|revert action replacement hooks
>   [6] livepatch: Do not enforce ELF_LIVEPATCH_FUNC section presence
>   [7] livepatch: Add per-function applied/reverted state tracking marker

I've added the test-cases to the little tool I use (including the diff)
http://xenbits.xen.org/gitweb/?p=xentesttools/bootstrap.git;a=blob;f=root_image/debugspace/livepatch_test.pl;h=37fb668a53ca1e7a084bfc4417f90e8ae078f7e5;hb=HEAD


>   [8] livepatch: Add support for inline asm hotpatching expectations

..but didn't expand on #8 as it still needs a bit of help on ARM..

Irrespective of that:

a)  Need to update the docs to alter the text where it says that the
entries are 8 bytes long - as on ARM32 they are 4 bytes.

b) Update the docs to say it is spec 2, not 1.

c) Also need OK or Acked-by from Julie on ARM.

I can do a) and b) when v3 is posted or when the fixes for to patch #8 are
all good and can squash them in. (And will also update the test-case to
include the code change for the asm livepatch).

diff --git a/root_image/debugspace/livepatch_test.pl b/root_image/debugspace/livepatch_test.pl
index 37fb668..a96c9fc 100755
--- a/root_image/debugspace/livepatch_test.pl
+++ b/root_image/debugspace/livepatch_test.pl
@@ -9,7 +9,14 @@ use File::Temp qw(tempfile);
 my @livepatch_files = qw(xen_hello_world.livepatch
 			 xen_replace_world.livepatch
                          xen_bye_world.livepatch
-			 xen_nop.livepatch);
+			 xen_nop.livepatch
+			 xen_no_xen_buildid.livepatch
+			 xen_prepost_hooks_fail.livepatch
+			 xen_prepost_hooks.livepatch
+			 xen_action_hooks.livepatch
+			 xen_action_hooks_marker.livepatch
+			 xen_action_hooks_noapply.livepatch
+			 xen_action_hooks_norevert.livepatch);
 
 my $livepatch_dir="/usr/lib/debug/livepatch";
 my $xen_extra_info;
@@ -111,6 +118,25 @@ my @livepatch_tests = (
     { C => "xen-livepatch unload xen_nop", rc => 256 },
     { C => "xen-livepatch revert xen_nop" },
     { C => "xen-livepatch unload xen_nop" },
+    { C => "xen-livepatch load xen_no_xen_buildid.livepatch", rc => 256 },
+    { C => "xen-livepatch load xen_prepost_hooks_fail.livepatch", rc => 256 },
+    { C => "xen-livepatch load xen_prepost_hooks.livepatch" },
+    # First revert fails
+    { C => "xen-livepatch revert xen_prepost_hooks", rc => 256 },
+    { C => "xen-livepatch revert xen_prepost_hooks" },
+    { C => "xen-livepatch unload xen_prepost_hooks" },
+    { C => "xen-livepatch load xen_action_hooks.livepatch" },
+    { C => "xen-livepatch revert xen_action_hooks" },
+    { C => "xen-livepatch unload xen_action_hooks" },
+    { C => "xen-livepatch load xen_action_hooks_marker.livepatch" },
+    { C => "xen-livepatch revert xen_action_hooks_marker" },
+    { C => "xen-livepatch unload xen_action_hooks_marker" },
+    { C => "xen-livepatch load xen_action_hooks_noapply.livepatch" },
+    { C => "xen-livepatch revert xen_action_hooks_noapply" },
+    { C => "xen-livepatch unload xen_action_hooks_noapply" },
+    { C => "xen-livepatch load xen_action_hooks_norevert.livepatch" },
+    { C => "xen-livepatch revert xen_action_hooks_norevert" },
+    { C => "xen-livepatch unload xen_action_hooks_norevert" },
     );
 
 # Copied from https://stackoverflow.com/questions/11514947/capture-the-output-of-perl-system
Konrad Rzeszutek Wilk Sept. 5, 2019, 7:13 p.m. UTC | #2
On Tue, Aug 27, 2019 at 08:46:12AM +0000, Pawel Wieczorkiewicz wrote:
> This series introduces new features to the livepatch functionality as
> briefly discussed during Xen Developer Summit 2019: [a] and [b].
> It also provides a few fixes and some small improvements.
> 
> Main changes in v2:
> - added new features to livepatch documentation
> - added livepatch tests
> - enabled Arm support for [5]
> - make .modinfo optional for [11]
> - fixed typos

I took your patches, redid them per what I had responded on these patches
(and squashed your fix for xen_expectations) and stuck them in my branch:

http://xenbits.xen.org/gitweb/?p=people/konradwilk/xen.git;a=shortlog;h=refs/heads/livepatch.aws.v3

There are three extra patches that were needed for me to test on ARM32 - those
are known issues and they don't block your patches. I will post them independent
of your patches.

From my perspective, your patches are good to go.

But I believe I need:
 - the tools folks Ack on the libxc code changes,
 - and also an Ack from the REST on sysctl.h,
 - and Julian OK on the ARM32/ARM64 code changes which are tiny, but nonethless are his.

Pawel, If I don't get to send them out by the end of the next week - feel free
to grab them from my branch tree and repost them as v3.

Thank you!
Julien Grall Sept. 6, 2019, 10:42 p.m. UTC | #3
Hi,

Thank you for the new version. However, I nearly missed the v2 as this 
is a sub-thread of v1. May I ask you to send a new version as a new 
thread instead?

Cheers,

On 8/27/19 9:46 AM, Pawel Wieczorkiewicz wrote:
> This series introduces new features to the livepatch functionality as
> briefly discussed during Xen Developer Summit 2019: [a] and [b].
> It also provides a few fixes and some small improvements.
> 
> Main changes in v2:
> - added new features to livepatch documentation
> - added livepatch tests
> - enabled Arm support for [5]
> - make .modinfo optional for [11]
> - fixed typos
> 
> FEATURES:
> 
> 1. independent modules (patches: [1], [2])
> 
>    * livepatch-build-tools repo dependency [A]
> 
>    Livepatch enforces the following buildid-based dependency chain
>    between hotpatch modules:
>      1) first module depends on given hypervisor buildid
>      2) every consecutive module depends on previous module's buildid
>    This way proper hotpatch stack order is maintained and enforced.
>    While it is important for production hotpatches it limits agility and
>    blocks usage of testing or debug hotpatches. These kinds of hotpatch
>    modules are typically expected to be loaded at any time irrespective
>    of current state of the modules stack.
> 
>    [A] livepatch-build: Embed hypervisor build id into every hotpatch
> 
> 2. pre- and post- apply|revert actions hooks (patches: [3], [4])
> 
>    * livepatch-build-tools repo dependency [B]
> 
>    This is an implementation of 4 new livepatch module vetoing hooks,
>    that can be optionally supplied along with modules.
>    Hooks that currently exists in the livepatch mechanism aren't agile
>    enough and have various limitations:
>    * run only from within a quiescing zone
>    * cannot conditionally prevent applying or reverting
>    * do not have access to the module context
>    To address these limitations the following has been implemented:
>    1) pre-apply hook
>    2) post-apply hook
>    3) pre-revert hook
>    4) post-revert hook
> 
>    [B] create-diff-object: Handle extra pre-|post- hooks
> 
> 3. apply|revert actions replacement hooks (patches: [5], [6], [7])
> 
>    * livepatch-build-tools repo dependency: [C], [D], [E]
> 
>    To increase hotpatching system's agility and provide more flexiable
>    long-term hotpatch solution, allow to overwrite the default apply
>    and revert action functions with hook-like supplied alternatives.
>    The alternative functions are optional and the default functions are
>    used by default.
> 
>    [C] create-diff-object: Do not create empty .livepatch.funcs section
>    [D] create-diff-object: Handle optional apply|revert hooks
>    [E] create-diff-object: Add support for applied/reverted marker
> 
> 4. inline asm hotpatching expectations (patches: [8])
> 
>    * livepatch-build-tools repo dependency: [F]
> 
>    Expectations are designed as optional feature, since the main use of
>    them is planned for inline asm hotpatching.
>    The payload structure is modified as each expectation structure is
>    part of the livepatch_func structure and hence extends the payload.
>    The payload version is bumped to 3 with this change to highlight the
>    ABI modification and enforce proper support.
>    The expectation is manually enabled during inline asm module
>    construction. If enabled, expectation ensures that the expected
>    content of memory is to be found at a given patching (old_addr)
>    location.
> 
>    [F] create-diff-object: Add support for expectations
> 
> 5. runtime hotpatch metadata support (patches: [9], [10], [11])
> 
>    Having detailed hotpatch metadata helps to properly identify module's
>    origin and version. It also allows to keep track of the history of
>    hotpatch loads in the system (at least within dmesg buffer size
>    limits).
>    Extend the livepatch list operation to fetch also payloads' metadata.
>    This is achieved by extending the sysctl list interface with 2 extra
>    guest handles:
>    * metadata     - an array of arbitrary size strings
>    * metadata_len - an array of metadata strings' lengths (uin32_t each)
>    To unify and simplify the interface, handle the modules' name strings
>    of arbitrary size by copying them in adhering chunks to the userland.
> 
> 6. python bindings for livepatch operations (patches: [12])
> 
>    Extend the XC python bindings library to support all common livepatch
>    operations and actions:
>    - status (pyxc_livepatch_status):
>    - action (pyxc_livepatch_action):
>    - upload (pyxc_livepatch_upload):
>    - list (pyxc_livepatch_list):
> 
> [a] https://wiki.xenproject.org/wiki/Design_Sessions_2019#LivePatch_improvements_and_features
> [b] https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg00846.html
> 
> Merged in v1:
>    python: Add XC binding for Xen build ID
>    livepatch: always print XENLOG_ERR information
> 
> Pawel Wieczorkiewicz (12):
>    [1] livepatch: Always check hypervisor build ID upon hotpatch upload
>    [2] livepatch: Allow to override inter-modules buildid dependency
>    [3] livepatch: Export payload structure via livepatch_payload.h
>    [4] livepatch: Implement pre-|post- apply|revert hooks
>    [5] livepatch: Add support for apply|revert action replacement hooks
>    [6] livepatch: Do not enforce ELF_LIVEPATCH_FUNC section presence
>    [7] livepatch: Add per-function applied/reverted state tracking marker
>    [8] livepatch: Add support for inline asm hotpatching expectations
>    [9] livepatch: Add support for modules .modinfo section metadata
>    [10] livepatch: Handle arbitrary size names with the list operation
>    [11] livepatch: Add metadata runtime retrieval mechanism
>    [12] livepatch: Add python bindings for livepatch operations
> 
>   .gitignore                                     |   6 +-
>   docs/misc/livepatch.pandoc                     | 231 ++++++++-
>   tools/libxc/include/xenctrl.h                  |  68 ++-
>   tools/libxc/xc_misc.c                          | 162 +++++--
>   tools/misc/xen-livepatch.c                     | 258 +++++++---
>   tools/python/xen/lowlevel/xc/xc.c              | 273 +++++++++++
>   xen/arch/arm/arm32/livepatch.c                 |  12 +-
>   xen/arch/arm/arm64/livepatch.c                 |  12 +-
>   xen/arch/arm/livepatch.c                       |  10 +-
>   xen/arch/x86/livepatch.c                       |  22 +-
>   xen/common/livepatch.c                         | 647 +++++++++++++++++++++----
>   xen/include/public/sysctl.h                    |  61 ++-
>   xen/include/xen/livepatch.h                    |  42 +-
>   xen/include/xen/livepatch_payload.h            |  83 ++++
>   xen/test/livepatch/Makefile                    | 113 ++++-
>   xen/test/livepatch/xen_action_hooks.c          | 102 ++++
>   xen/test/livepatch/xen_action_hooks_marker.c   | 112 +++++
>   xen/test/livepatch/xen_action_hooks_noapply.c  | 136 ++++++
>   xen/test/livepatch/xen_action_hooks_nofunc.c   |  86 ++++
>   xen/test/livepatch/xen_action_hooks_norevert.c | 143 ++++++
>   xen/test/livepatch/xen_expectations.c          |  41 ++
>   xen/test/livepatch/xen_expectations_fail.c     |  42 ++
>   xen/test/livepatch/xen_prepost_hooks.c         | 122 +++++
>   xen/test/livepatch/xen_prepost_hooks_fail.c    |  75 +++
>   24 files changed, 2579 insertions(+), 280 deletions(-)
>   create mode 100644 xen/test/livepatch/xen_action_hooks.c
>   create mode 100644 xen/test/livepatch/xen_action_hooks_marker.c
>   create mode 100644 xen/test/livepatch/xen_action_hooks_noapply.c
>   create mode 100644 xen/test/livepatch/xen_action_hooks_nofunc.c
>   create mode 100644 xen/test/livepatch/xen_action_hooks_norevert.c
>   create mode 100644 xen/test/livepatch/xen_expectations.c
>   create mode 100644 xen/test/livepatch/xen_expectations_fail.c
>   create mode 100644 xen/test/livepatch/xen_prepost_hooks.c
>   create mode 100644 xen/test/livepatch/xen_prepost_hooks_fail.c
>
Julien Grall Sept. 6, 2019, 10:52 p.m. UTC | #4
Hi Konrad,

On 9/5/19 8:13 PM, Konrad Rzeszutek Wilk wrote:
> On Tue, Aug 27, 2019 at 08:46:12AM +0000, Pawel Wieczorkiewicz wrote:
>> This series introduces new features to the livepatch functionality as
>> briefly discussed during Xen Developer Summit 2019: [a] and [b].
>> It also provides a few fixes and some small improvements.
>>
>> Main changes in v2:
>> - added new features to livepatch documentation
>> - added livepatch tests
>> - enabled Arm support for [5]
>> - make .modinfo optional for [11]
>> - fixed typos
> 
> I took your patches, redid them per what I had responded on these patches
> (and squashed your fix for xen_expectations) and stuck them in my branch:
> 
> http://xenbits.xen.org/gitweb/?p=people/konradwilk/xen.git;a=shortlog;h=refs/heads/livepatch.aws.v3
> 
> There are three extra patches that were needed for me to test on ARM32 - those
> are known issues and they don't block your patches. I will post them independent
> of your patches.
> 
>  From my perspective, your patches are good to go.
> 
> But I believe I need:
>   - the tools folks Ack on the libxc code changes,
>   - and also an Ack from the REST on sysctl.h,
>   - and Julian OK on the ARM32/ARM64 code changes which are tiny, but nonethless are his.

I believe all the modifications are under in livepatch.c files. So your 
Ack should be sufficient here :).

Anyway, feel free to add mine on Arm specific modifications.

Cheers,