[Xen-devel] [PATCH v2 0/7] livepatch-build-tools: new features and fixes

[Xen-devel] [PATCH v2 0/7] livepatch-build-tools: new features and fixes

[Pawel Wieczorkiewicz]

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.

IMPROVEMENTS:

1. Strip redundant or transient symbols from resulting object files:
   [6], [7]

  This change limits the size of resulting modules by dropping unneeded
  bits.

FEATURES:

For details see cover letter of xen series: [c]

1. independent modules (patches: [1])
2. pre- and post- apply|revert actions hooks (patches: [2])
3. apply|revert actions replacement hooks (patches: [3], [4])
4. inline asm hotpatching expectations (patches: [5])


[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

[c] [PATCH 00/14] livepatch: new features and fixes

Merged in v1:
  livepatch-gcc: Allow toolchain command with versions
  livepatch-gcc: Ignore built_in.o and prelink.o object files
  livepatch-build: Do not follow every symlink for patch file
  livepatch-build: Handle newly created object files
  common: Add is_standard_section() helper function
  common: Add is_referenced_section() helper function
  create-diff-object: Add is_special_section() helper function
  livepatch-build: detect special section group sizes
  create-diff-object: Add new entries to special sections array
  create-diff-object: Do not include all .rodata sections
  create-diff-object: Do not create empty .livepatch.funcs section
  create-diff-object: Extend patchability verification: STN_UNDEF
  create-diff-object: Strip all undefined entires of known size

Pawel Wieczorkiewicz (7):
  [1] livepatch-build: Embed hypervisor build id into every hotpatch
  [2] create-diff-object: Handle extra pre-|post- hooks
  [3] create-diff-object: Handle optional apply|revert hooks
  [4] create-diff-object: Add support for applied/reverted marker
  [5] create-diff-object: Add support for expectations
  [6] livepatch-build: Strip transient or unneeded symbols
  [7] livepatch-build: Strip all metadata symbols from hotpatch modules

 common.h             | 13 +++++++-
 create-diff-object.c | 82 ++++++++++++++++++++++++++++++++++++++--------
 livepatch-build      | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 171 insertions(+), 16 deletions(-)

-- 
2.16.5




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 1/7] livepatch-build: Embed hypervisor build id into every hotpatch

[Pawel Wieczorkiewicz]

This change is part of a independant stacked hotpatch modules
feature. This feature allows to bypass dependencies between modules
upon loading, but still verifies Xen build ID matching.

With stacked hotpatch modules it is essential that each and every
hotpatch is verified against the hypervisor build id upon upload.
It must not be possible to successfully upload hotpatches built for
incorrect version of the hypervisor.

To achieve that always embed an additional ELF section:
'.livpatch.xen_depends' containing the hypervisor build id.

The hypervisor build id must be always provided as a command line
parameter: --xen-depends.

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
Reviewed-by: Bjoern Doebel <doebel@amazon.de>
Reviewed-by: Norbert Manthey <nmanthey@amazon.de>
---
 livepatch-build | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/livepatch-build b/livepatch-build
index b198c97..b8a1728 100755
--- a/livepatch-build
+++ b/livepatch-build
@@ -30,6 +30,7 @@ DEBUG=n
 XEN_DEBUG=n
 SKIP=
 DEPENDS=
+XEN_DEPENDS=
 PRELINK=
 XENSYMS=xen-syms
 
@@ -163,6 +164,9 @@ function create_patch()
     # Create a dependency section
     perl -e "print pack 'VVVZ*H*', 4, 20, 3, 'GNU', '${DEPENDS}'" > depends.bin
 
+    # Create a Xen dependency section
+    perl -e "print pack 'VVVZ*H*', 4, 20, 3, 'GNU', '${XEN_DEPENDS}'" > xen_depends.bin
+
     echo "Creating patch module..."
     if [ -z "$PRELINK" ]; then
         ld -r -o "${PATCHNAME}.livepatch" --build-id=sha1 $(find output -type f -name "*.o") || die
@@ -174,6 +178,9 @@ function create_patch()
 
     objcopy --add-section .livepatch.depends=depends.bin "${PATCHNAME}.livepatch"
     objcopy --set-section-flags .livepatch.depends=alloc,readonly "${PATCHNAME}.livepatch"
+
+    objcopy --add-section .livepatch.xen_depends=xen_depends.bin "${PATCHNAME}.livepatch"
+    objcopy --set-section-flags .livepatch.xen_depends=alloc,readonly "${PATCHNAME}.livepatch"
 }
 
 usage() {
@@ -189,12 +196,13 @@ usage() {
     echo "        --xen-debug        Build debug Xen (if your .config does not have the options)" >&2
     echo "        --xen-syms         Build against a xen-syms" >&2
     echo "        --depends          Required build-id" >&2
+    echo "        --xen-depends      Required Xen build-id" >&2
     echo "        --prelink          Prelink" >&2
 }
 
 find_tools || die "can't find supporting tools"
 
-options=$(getopt -o hs:p:c:o:j:k:d -l "help,srcdir:,patch:,config:,output:,cpus:,skip:,debug,xen-debug,xen-syms:,depends:,prelink" -- "$@") || die "getopt failed"
+options=$(getopt -o hs:p:c:o:j:k:d -l "help,srcdir:,patch:,config:,output:,cpus:,skip:,debug,xen-debug,xen-syms:,depends:,xen-depends:,prelink" -- "$@") || die "getopt failed"
 
 eval set -- "$options"
 
@@ -253,6 +261,11 @@ while [[ $# -gt 0 ]]; do
             DEPENDS="$1"
             shift
             ;;
+        --xen-depends)
+            shift
+            XEN_DEPENDS="$1"
+            shift
+            ;;
         --prelink)
             PRELINK=--resolve
             shift
@@ -269,6 +282,7 @@ done
 [ -z "$configarg" ] && die ".config not given"
 [ -z "$outputarg" ] && die "Output directory not given"
 [ -z "$DEPENDS" ] && die "Build-id dependency not given"
+[ -z "$XEN_DEPENDS" ] && die "Xen Build-id dependency not given"
 
 SRCDIR="$(readlink -m -- "$srcarg")"
 # We need an absolute path because we move around, but we need to
-- 
2.16.5




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 2/7] create-diff-object: Handle extra pre-|post- hooks

[Pawel Wieczorkiewicz]

Include new sections containing optional pre-, post- action hooks.

The following new section names are supported:
  - .livepatch.hooks.preapply
  - .livepatch.hooks.postapply
  - .livepatch.hooks.prerevert
  - .livepatch.hooks.postrevert

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 create-diff-object.c | 67 ++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 54 insertions(+), 13 deletions(-)

diff --git a/create-diff-object.c b/create-diff-object.c
index f01f1da..44f4e6b 100644
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -1102,6 +1102,22 @@ static struct special_section special_sections[] = {
 		.name		= ".livepatch.hooks.unload",
 		.group_size	= livepatch_hooks_group_size,
 	},
+	{
+		.name		= ".livepatch.hooks.preapply",
+		.group_size	= livepatch_hooks_group_size,
+	},
+	{
+		.name		= ".livepatch.hooks.postapply",
+		.group_size	= livepatch_hooks_group_size,
+	},
+	{
+		.name		= ".livepatch.hooks.prerevert",
+		.group_size	= livepatch_hooks_group_size,
+	},
+	{
+		.name		= ".livepatch.hooks.postrevert",
+		.group_size	= livepatch_hooks_group_size,
+	},
 	{},
 };
 
@@ -1465,23 +1481,44 @@ static void kpatch_include_debug_sections(struct kpatch_elf *kelf)
 	}
 }
 
-static void kpatch_include_hook_elements(struct kpatch_elf *kelf)
+#define IS_HOOK_SECTION(section, hook) ({ \
+        !strcmp(((section))->name, ".livepatch.hooks." hook) || \
+        !strcmp(((section))->name, ".rela.livepatch.hooks." hook); \
+})
+
+#define IS_ACTION_HOOK_SECTION(section, action) ({ \
+        IS_HOOK_SECTION(section, "pre" action) || \
+        IS_HOOK_SECTION(section, "post" action); \
+})
+
+#define IS_HOOK_SYM_NAME(symbol, hook) ({ \
+        !strcmp(((symbol))->name, "livepatch_" hook "_data"); \
+})
+
+#define IS_ACTION_HOOK_SYM_NAME(symbol, action) ({ \
+        IS_HOOK_SYM_NAME(symbol, "pre" action) || \
+        IS_HOOK_SYM_NAME(symbol, "post" action); \
+})
+
+static int kpatch_include_hook_elements(struct kpatch_elf *kelf)
 {
 	struct section *sec;
 	struct symbol *sym;
 	struct rela *rela;
+	int num_new_functions = 0;
 
-	/* include load/unload sections */
+	/* include all supported hooks sections */
 	list_for_each_entry(sec, &kelf->sections, list) {
-		if (!strcmp(sec->name, ".livepatch.hooks.load") ||
-		    !strcmp(sec->name, ".livepatch.hooks.unload") ||
-		    !strcmp(sec->name, ".rela.livepatch.hooks.load") ||
-		    !strcmp(sec->name, ".rela.livepatch.hooks.unload")) {
+		if (IS_HOOK_SECTION(sec, "load") ||
+		    IS_HOOK_SECTION(sec, "unload") ||
+		    IS_ACTION_HOOK_SECTION(sec, "apply") ||
+		    IS_ACTION_HOOK_SECTION(sec, "revert")) {
 			sec->include = 1;
+			num_new_functions++;
 			if (is_rela_section(sec)) {
 				/* include hook dependencies */
 				rela = list_entry(sec->relas.next,
-			                         struct rela, list);
+						  struct rela, list);
 				sym = rela->sym;
 				log_normal("found hook: %s\n",sym->name);
 				kpatch_include_symbol(sym, 0);
@@ -1497,13 +1534,17 @@ static void kpatch_include_hook_elements(struct kpatch_elf *kelf)
 	}
 
 	/*
-	 * Strip temporary global load/unload function pointer objects
-	 * used by the kpatch_[load|unload]() macros.
+	 * Strip temporary global function pointer objects for all
+	 * supported hooks, used by the kpatch_[load|unload]() macros.
 	 */
 	list_for_each_entry(sym, &kelf->symbols, list)
-		if (!strcmp(sym->name, "livepatch_load_data") ||
-		    !strcmp(sym->name, "livepatch_unload_data"))
+		if (IS_HOOK_SYM_NAME(sym, "load") ||
+		    IS_HOOK_SYM_NAME(sym, "unload") ||
+		    IS_ACTION_HOOK_SYM_NAME(sym, "apply") ||
+		    IS_ACTION_HOOK_SYM_NAME(sym, "revert"))
 			sym->include = 0;
+
+	return num_new_functions;
 }
 
 static int kpatch_include_new_globals(struct kpatch_elf *kelf)
@@ -2292,11 +2333,11 @@ int main(int argc, char *argv[])
 	kpatch_include_standard_elements(kelf_patched);
 	log_debug("Include changed functions\n");
 	num_changed = kpatch_include_changed_functions(kelf_patched);
-	log_debug("num_changed = %d\n", num_changed);
 	log_debug("Include debug sections\n");
 	kpatch_include_debug_sections(kelf_patched);
 	log_debug("Include hook elements\n");
-	kpatch_include_hook_elements(kelf_patched);
+	num_changed += kpatch_include_hook_elements(kelf_patched);
+	log_debug("num_changed = %d\n", num_changed);
 	log_debug("Include standard string elements\n");
 	kpatch_include_standard_string_elements(kelf_patched);
 	log_debug("Include new globals\n");
-- 
2.16.5




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 3/7] create-diff-object: Handle optional apply|revert hooks

[Pawel Wieczorkiewicz]

Include new sections containing optional apply and revert action
hooks.

The following new section names are supported:
  - .livepatch.hooks.apply
  - .livepatch.hooks.revert

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
---
 create-diff-object.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/create-diff-object.c b/create-diff-object.c
index 44f4e6b..9918b6e 100644
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -1118,6 +1118,14 @@ static struct special_section special_sections[] = {
 		.name		= ".livepatch.hooks.postrevert",
 		.group_size	= livepatch_hooks_group_size,
 	},
+	{
+		.name		= ".livepatch.hooks.apply",
+		.group_size	= livepatch_hooks_group_size,
+	},
+	{
+		.name		= ".livepatch.hooks.revert",
+		.group_size	= livepatch_hooks_group_size,
+	},
 	{},
 };
 
@@ -1488,6 +1496,7 @@ static void kpatch_include_debug_sections(struct kpatch_elf *kelf)
 
 #define IS_ACTION_HOOK_SECTION(section, action) ({ \
         IS_HOOK_SECTION(section, "pre" action) || \
+        IS_HOOK_SECTION(section, action) || \
         IS_HOOK_SECTION(section, "post" action); \
 })
 
@@ -1497,6 +1506,7 @@ static void kpatch_include_debug_sections(struct kpatch_elf *kelf)
 
 #define IS_ACTION_HOOK_SYM_NAME(symbol, action) ({ \
         IS_HOOK_SYM_NAME(symbol, "pre" action) || \
+        IS_HOOK_SYM_NAME(symbol, action) || \
         IS_HOOK_SYM_NAME(symbol, "post" action); \
 })
 
-- 
2.16.5




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 4/7] create-diff-object: Add support for applied/reverted marker

[Pawel Wieczorkiewicz]

With version 2 of a payload structure additional field is supported
to track whether given function has been applied or reverted.
There also comes additional 8-byte alignment padding to reserve
place for future flags and options.

The new fields are zero-out upon .livepatch.funcs section creation.

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
---
 common.h             | 2 ++
 create-diff-object.c | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/common.h b/common.h
index 06e19e7..d8cde35 100644
--- a/common.h
+++ b/common.h
@@ -124,6 +124,8 @@ struct livepatch_patch_func {
 	uint32_t old_size;
 	uint8_t version;
 	unsigned char pad[31];
+	uint8_t applied;
+	uint8_t _pad[7];
 };
 
 struct special_section {
diff --git a/create-diff-object.c b/create-diff-object.c
index 9918b6e..d518503 100644
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -2014,8 +2014,10 @@ static void livepatch_create_patches_sections(struct kpatch_elf *kelf,
 			funcs[index].old_size = result.size;
 			funcs[index].new_addr = 0;
 			funcs[index].new_size = sym->sym.st_size;
-			funcs[index].version = 1;
+			funcs[index].version = 2;
 			memset(funcs[index].pad, 0, sizeof funcs[index].pad);
+			funcs[index].applied = 0;
+			memset(funcs[index]._pad, 0, sizeof funcs[index]._pad);
 
 			/*
 			 * Add a relocation that will populate
-- 
2.16.5




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 5/7] create-diff-object: Add support for expectations

[Pawel Wieczorkiewicz]

Extend livepatch_patch_func to support a new field: expect. This new
field describes the expected data, its length and whether expectation
is enabled. The expectation's data is of opaque padding size.

By default the expectation field is zero-out and the expectation is
disabled unless explicitly specified in the patch.

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
---
Changed since v1:
  * Do not bump the payload version again.

 common.h             | 11 ++++++++++-
 create-diff-object.c |  1 +
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/common.h b/common.h
index d8cde35..7c6fb73 100644
--- a/common.h
+++ b/common.h
@@ -115,6 +115,14 @@ struct kpatch_elf {
 };
 
 #define PATCH_INSN_SIZE 5
+#define MAX_REPLACEMENT_SIZE 31
+struct livepatch_expectation {
+	uint8_t enabled : 1;
+	uint8_t len : 5;
+	uint8_t pad : 2;
+	uint8_t data[MAX_REPLACEMENT_SIZE];
+};
+typedef struct livepatch_expectation livepatch_expectation_t;
 
 struct livepatch_patch_func {
 	char *name;
@@ -123,9 +131,10 @@ struct livepatch_patch_func {
 	uint32_t new_size;
 	uint32_t old_size;
 	uint8_t version;
-	unsigned char pad[31];
+	unsigned char pad[MAX_REPLACEMENT_SIZE];
 	uint8_t applied;
 	uint8_t _pad[7];
+	livepatch_expectation_t expect;
 };
 
 struct special_section {
diff --git a/create-diff-object.c b/create-diff-object.c
index d518503..e4592a6 100644
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -2018,6 +2018,7 @@ static void livepatch_create_patches_sections(struct kpatch_elf *kelf,
 			memset(funcs[index].pad, 0, sizeof funcs[index].pad);
 			funcs[index].applied = 0;
 			memset(funcs[index]._pad, 0, sizeof funcs[index]._pad);
+			memset(&funcs[index].expect, 0, sizeof funcs[index].expect);
 
 			/*
 			 * Add a relocation that will populate
-- 
2.16.5




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 6/7] livepatch-build: Strip transient or unneeded symbols

[Pawel Wieczorkiewicz]

In the process of creating a final hotpatch module file make sure to
strip all transient symbols that have not been caught and removed by
create-diff-object processing. For now these are only the hooks
kpatch load/unload symbols.

For all new object files that are carried along for the final linking
the transient hooks symbols are not stripped and neither are any
unneeded symbols. Strip them explicitly from resulting object file.

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
---
 livepatch-build | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/livepatch-build b/livepatch-build
index b8a1728..816064c 100755
--- a/livepatch-build
+++ b/livepatch-build
@@ -111,6 +111,28 @@ function build_special()
     unset LIVEPATCH_CAPTURE_DIR
 }
 
+strip_extra_symbols ()
+{
+    local -r FILE="$1"
+    local -a STRIP_CMD_OPTS=()
+    local -a SYM_PREFIX=("livepatch_load_data_"
+                         "livepatch_unload_data_"
+                         "livepatch_preapply_data_"
+                         "livepatch_apply_data_"
+                         "livepatch_postapply_data_"
+                         "livepatch_prerevert_data_"
+                         "livepatch_revert_data_"
+                         "livepatch_postrevert_data_")
+
+    STRIP_CMD_OPTS+=("-w")
+    for sym in "${SYM_PREFIX[@]}"; do
+        STRIP_CMD_OPTS+=("-N")
+        STRIP_CMD_OPTS+=("\"${sym}*\"")
+    done
+
+    strip "${STRIP_CMD_OPTS[@]}" "$FILE"
+}
+
 function create_patch()
 {
     echo "Extracting new and modified ELF sections..."
@@ -150,6 +172,7 @@ function create_patch()
     NEW_FILES=$(comm -23 <(cd patched/xen && find . -type f -name '*.o' | sort) <(cd original/xen && find . -type f -name '*.o' | sort))
     for i in $NEW_FILES; do
         cp "patched/$i" "output/$i"
+        strip --strip-unneeded "output/$i"
         CHANGED=1
     done
 
@@ -176,6 +199,8 @@ function create_patch()
         "${TOOLSDIR}"/prelink $debugopt output.o "${PATCHNAME}.livepatch" "$XENSYMS" &>> "${OUTPUT}/prelink.log" || die
     fi
 
+    strip_extra_symbols "${PATCHNAME}.livepatch"
+
     objcopy --add-section .livepatch.depends=depends.bin "${PATCHNAME}.livepatch"
     objcopy --set-section-flags .livepatch.depends=alloc,readonly "${PATCHNAME}.livepatch"
 
-- 
2.16.5




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 7/7] livepatch-build: Strip all metadata symbols from hotpatch modules

[Pawel Wieczorkiewicz]

Strip all unneeded metadata symbols from generated hotpatch modules.
The metadata symbols are the symbols from metadata-like sections (e.g.
'.livepatch.funcs') or livepatch hooks symbols (defined by a set of
prefixes. E.g. 'livepatch_load_data_').

By default the create-diff-object does not create symbols in metadata
sections. However, such symbols may be implicitly added by speciying
extra entries in the sections manually (in a given patch).
The symbols are not needed for the hotpatch modules and should be
stripped to avoid symbol names collisions and to save hotpatch files
space.

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
---
 livepatch-build | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 2 deletions(-)

diff --git a/livepatch-build b/livepatch-build
index 816064c..be459c9 100755
--- a/livepatch-build
+++ b/livepatch-build
@@ -111,10 +111,39 @@ function build_special()
     unset LIVEPATCH_CAPTURE_DIR
 }
 
-strip_extra_symbols ()
+elf_section_exists ()
+{
+    local ELF="$1"
+    local SEC="$2"
+
+    objdump -h -j "$SEC" "$ELF" &> /dev/null
+}
+
+# Extract a set of unique symbols for a specified section.
+elf_extract_section_symbols ()
+{
+    local -r ELF="$1"
+    local -r SEC="$2"
+
+    if elf_section_exists "$ELF" "$SEC"
+    then
+        # Example objdump command output to be parsed:
+        #
+        # SYMBOL TABLE:
+        # 0000000000000000 l    d  .livepatch.funcs    0000000000000000 .livepatch.funcs
+        objdump -w -j "$SEC" -t "$ELF" | awk '/^SYMBOL TABLE:/ {seen = 1; next} seen && $NF {print $NF}' | sort -u
+    fi
+}
+
+# Strip all metadata symbols belonging to a metadata section
+# or whose name starts with a livepatch hook prefix.
+# The function constructs the 'strip' utility command line
+# and then invokes strip with that command line.
+strip_metadata_symbols ()
 {
     local -r FILE="$1"
     local -a STRIP_CMD_OPTS=()
+    local -a SYM_SECTIONS=(".livepatch.funcs")
     local -a SYM_PREFIX=("livepatch_load_data_"
                          "livepatch_unload_data_"
                          "livepatch_preapply_data_"
@@ -123,13 +152,35 @@ strip_extra_symbols ()
                          "livepatch_prerevert_data_"
                          "livepatch_revert_data_"
                          "livepatch_postrevert_data_")
+    local -a SYMS=()
 
+    # Enable wildcard
     STRIP_CMD_OPTS+=("-w")
+
+    # Strip all livepatch hooks metadata symbols
     for sym in "${SYM_PREFIX[@]}"; do
         STRIP_CMD_OPTS+=("-N")
         STRIP_CMD_OPTS+=("\"${sym}*\"")
     done
 
+    # Find all symbols from metadata sections
+    # Note: There may be name conflicts between global
+    # and local symbols belonging to the same section.
+    # For the '.livepatch.funcs' section it is not a
+    # problem. Think about it before adding more sections.
+    for sec in "${SYM_SECTIONS[@]}"; do
+        SYMS+=($(elf_extract_section_symbols "$FILE" "$sec"))
+    done
+
+    # Strip metadata sections' symbols
+    if [ ${#SYMS[@]} -gt 0 ]
+    then
+        for sym in "${SYMS[@]}"; do
+            STRIP_CMD_OPTS+=("-N")
+            STRIP_CMD_OPTS+=("${sym}")
+        done
+    fi
+
     strip "${STRIP_CMD_OPTS[@]}" "$FILE"
 }
 
@@ -199,7 +250,7 @@ function create_patch()
         "${TOOLSDIR}"/prelink $debugopt output.o "${PATCHNAME}.livepatch" "$XENSYMS" &>> "${OUTPUT}/prelink.log" || die
     fi
 
-    strip_extra_symbols "${PATCHNAME}.livepatch"
+    strip_metadata_symbols "${PATCHNAME}.livepatch"
 
     objcopy --add-section .livepatch.depends=depends.bin "${PATCHNAME}.livepatch"
     objcopy --set-section-flags .livepatch.depends=alloc,readonly "${PATCHNAME}.livepatch"
-- 
2.16.5




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 1/7] livepatch-build: Embed hypervisor build id into every hotpatch

[Ross Lagerwall]

On 9/16/19 12:30 PM, Pawel Wieczorkiewicz wrote:
> This change is part of a independant stacked hotpatch modules
> feature. This feature allows to bypass dependencies between modules
> upon loading, but still verifies Xen build ID matching.
> 
> With stacked hotpatch modules it is essential that each and every
> hotpatch is verified against the hypervisor build id upon upload.
> It must not be possible to successfully upload hotpatches built for
> incorrect version of the hypervisor.
> 
> To achieve that always embed an additional ELF section:
> '.livpatch.xen_depends' containing the hypervisor build id.
> 
> The hypervisor build id must be always provided as a command line
> parameter: --xen-depends.
> 
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
> Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
> Reviewed-by: Bjoern Doebel <doebel@amazon.de>
> Reviewed-by: Norbert Manthey <nmanthey@amazon.de>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 3/7] create-diff-object: Handle optional apply|revert hooks

[Ross Lagerwall]

On 9/16/19 12:30 PM, Pawel Wieczorkiewicz wrote:
> Include new sections containing optional apply and revert action
> hooks.
> 
> The following new section names are supported:
>   - .livepatch.hooks.apply
>   - .livepatch.hooks.revert
> 
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 4/7] create-diff-object: Add support for applied/reverted marker

[Ross Lagerwall]

On 9/16/19 12:30 PM, Pawel Wieczorkiewicz wrote:
> With version 2 of a payload structure additional field is supported
> to track whether given function has been applied or reverted.
> There also comes additional 8-byte alignment padding to reserve
> place for future flags and options.
> 
> The new fields are zero-out upon .livepatch.funcs section creation.
> 
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>


Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 5/7] create-diff-object: Add support for expectations

[Ross Lagerwall]

On 9/16/19 12:30 PM, Pawel Wieczorkiewicz wrote:
> Extend livepatch_patch_func to support a new field: expect. This new
> field describes the expected data, its length and whether expectation
> is enabled. The expectation's data is of opaque padding size.
> 
> By default the expectation field is zero-out and the expectation is
> disabled unless explicitly specified in the patch.
> 
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 6/7] livepatch-build: Strip transient or unneeded symbols

[Ross Lagerwall]

On 9/16/19 12:30 PM, Pawel Wieczorkiewicz wrote:
> In the process of creating a final hotpatch module file make sure to
> strip all transient symbols that have not been caught and removed by
> create-diff-object processing. For now these are only the hooks
> kpatch load/unload symbols.
> 
> For all new object files that are carried along for the final linking
> the transient hooks symbols are not stripped and neither are any
> unneeded symbols. Strip them explicitly from resulting object file.
> 
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
> ---
>  livepatch-build | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/livepatch-build b/livepatch-build
> index b8a1728..816064c 100755
> --- a/livepatch-build
> +++ b/livepatch-build
> @@ -111,6 +111,28 @@ function build_special()
>      unset LIVEPATCH_CAPTURE_DIR
>  }
>  
> +strip_extra_symbols ()
> +{
> +    local -r FILE="$1"
> +    local -a STRIP_CMD_OPTS=()
> +    local -a SYM_PREFIX=("livepatch_load_data_"
> +                         "livepatch_unload_data_"
> +                         "livepatch_preapply_data_"
> +                         "livepatch_apply_data_"
> +                         "livepatch_postapply_data_"
> +                         "livepatch_prerevert_data_"
> +                         "livepatch_revert_data_"
> +                         "livepatch_postrevert_data_")
> +
> +    STRIP_CMD_OPTS+=("-w")
> +    for sym in "${SYM_PREFIX[@]}"; do
> +        STRIP_CMD_OPTS+=("-N")
> +        STRIP_CMD_OPTS+=("\"${sym}*\"")
> +    done
> +
> +    strip "${STRIP_CMD_OPTS[@]}" "$FILE"
> +}
> +
>  function create_patch()
>  {
>      echo "Extracting new and modified ELF sections..."
> @@ -150,6 +172,7 @@ function create_patch()
>      NEW_FILES=$(comm -23 <(cd patched/xen && find . -type f -name '*.o' | sort) <(cd original/xen && find . -type f -name '*.o' | sort))
>      for i in $NEW_FILES; do
>          cp "patched/$i" "output/$i"
> +        strip --strip-unneeded "output/$i"

This strips debug symbols too which is not necessarily desirable and I think for most software is normally left a high level process (e.g. rpmbuild). Can you make this optional please?

Thanks,
-- 
Ross Lagerwall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 7/7] livepatch-build: Strip all metadata symbols from hotpatch modules

[Ross Lagerwall]

On 9/16/19 12:30 PM, Pawel Wieczorkiewicz wrote:
> Strip all unneeded metadata symbols from generated hotpatch modules.
> The metadata symbols are the symbols from metadata-like sections (e.g.
> '.livepatch.funcs') or livepatch hooks symbols (defined by a set of
> prefixes. E.g. 'livepatch_load_data_').
> 
> By default the create-diff-object does not create symbols in metadata
> sections. However, such symbols may be implicitly added by speciying
> extra entries in the sections manually (in a given patch).
> The symbols are not needed for the hotpatch modules and should be
> stripped to avoid symbol names collisions and to save hotpatch files
> space.
> 
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 6/7] livepatch-build: Strip transient or unneeded symbols

[Wieczorkiewicz, Pawel]

> On 25. Nov 2019, at 15:38, Ross Lagerwall <ross.lagerwall@citrix.com> wrote:
> 
> On 9/16/19 12:30 PM, Pawel Wieczorkiewicz wrote:
>> In the process of creating a final hotpatch module file make sure to
>> strip all transient symbols that have not been caught and removed by
>> create-diff-object processing. For now these are only the hooks
>> kpatch load/unload symbols.
>> 
>> 

snip

>> function create_patch()
>> {
>>     echo "Extracting new and modified ELF sections..."
>> @@ -150,6 +172,7 @@ function create_patch()
>>     NEW_FILES=$(comm -23 <(cd patched/xen && find . -type f -name '*.o' | sort) <(cd original/xen && find . -type f -name '*.o' | sort))
>>     for i in $NEW_FILES; do
>>         cp "patched/$i" "output/$i"
>> +        strip --strip-unneeded "output/$i"
> 
> This strips debug symbols too which is not necessarily desirable and I think for most software is normally left a high level process (e.g. rpmbuild). Can you make this optional please?
> 

Yes, will do. Thanks for looking.

> Thanks,
> -- 
> Ross Lagerwall

Best Regards,
Pawel Wieczorkiewicz






Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel