[PATCH v4 07/11] livepatch/x86/arm[32, 64]: Force .livepatch.depends section to be uint32_t aligned.

[Konrad Rzeszutek Wilk <konrad@kernel.org>]

By default when using objcopy we lose the alignment when we copy it from xen-syms -
with the result that alignment (on ARM32 for example) can be 1:

  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
..
  [ 6] .livepatch.depend PROGBITS        00000000 000093 000024 00   A  0   0  1

That, combined with wacky offset means it will be loaded in
memory with the wrong alignment:

(XEN) livepatch.c:425: livepatch: xen_bye_world: Loaded .livepatch.depends at 000a08043

And later we crash as the .livepatch.depends is not aligned to four bytes, while
the xen_build_id_check expects the code to be four byte aligned and we
get an hypervisor crash (on ARM32):

(XEN) CPU0: Unexpected Trap: Data Abort
(XEN) ----[ Xen-4.10Hello World  arm32  debug=y   Not tainted ]----
(XEN) CPU:    0
(XEN) PC:     002400a0 xen_build_id_check+0x8/0xe8
..snip..
(XEN) Xen call trace:
(XEN)    [<002400a0>] xen_build_id_check+0x8/0xe8 (PC)
(XEN)    [<0021a9c0>] livepatch_op+0x768/0x1610 (LR)
(XEN)    [<0023bbe4>] do_sysctl+0x9c8/0xa9c
(XEN)    [<002673c4>] do_trap_guest_sync+0x11e0/0x177c
(XEN)    [<0026b6a0>] entry.o#return_from_trap+0/0x4
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) CPU0: Unexpected Trap: Data Abort

This fix forces all the test-cases to be built with a
.livepatch.depends structure containing the build-id extracted from
the hypervisor (except the xen_bye_world test-case).

We use the 'mkhex' tool instead of 'xxd' as the end result is an 'unsigned'
instead of 'char' type array - which naturally forces the alignment to be of four.
Also the 'mkhex' tools allows us to pass the section name as parameter.

The end result is much better alignment:

  [ 7] .livepatch.depend PROGBITS        00000000 000094 000024 00   A  0   0  4

Note that thanks to 'unsigned int .. __note_depends' the symbol becomes
global:

$ readelf --symbols *.livepatch | grep depen
    23: 0000000000000000    36 OBJECT  GLOBAL HIDDEN     6 note_depends
    49: 0000000000000000    36 OBJECT  GLOBAL HIDDEN    17 note_depends
    16: 0000000000000000    36 OBJECT  GLOBAL HIDDEN     3 note_depends
    21: 0000000000000000    36 OBJECT  GLOBAL HIDDEN     6 note_depends

See patch titled: "livepatch/arm/x86: Rename note_depends symbol from test-cases."
which fixes this.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>

v2: First posting.
v3: - Used mkhex from tools/misc instead of tools/firmware/hvmloader/
    - Include the XEN crash
---
 docs/misc/livepatch.markdown           |  2 ++
 xen/test/livepatch/Makefile            | 56 +++++++++++++++-------------------
 xen/test/livepatch/xen_bye_world.c     |  1 +
 xen/test/livepatch/xen_hello_world.c   |  1 +
 xen/test/livepatch/xen_nop.c           |  1 +
 xen/test/livepatch/xen_replace_world.c |  1 +
 6 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/docs/misc/livepatch.markdown b/docs/misc/livepatch.markdown
index 59f89aa292..091029781e 100644
--- a/docs/misc/livepatch.markdown
+++ b/docs/misc/livepatch.markdown
@@ -430,6 +430,8 @@ checksum, MD5 checksum or any unique value.
 
 The size of these structures varies with the --build-id linker option.
 
+On ARM32 this section must by four-byte aligned.
+
 ## Hypercalls
 
 We will employ the sub operations of the system management hypercall (sysctl).
diff --git a/xen/test/livepatch/Makefile b/xen/test/livepatch/Makefile
index 6831383db1..d23833e36f 100644
--- a/xen/test/livepatch/Makefile
+++ b/xen/test/livepatch/Makefile
@@ -1,15 +1,7 @@
 include $(XEN_ROOT)/Config.mk
 
-ifeq ($(XEN_TARGET_ARCH),x86_64)
-OBJCOPY_MAGIC := -I binary -O elf64-x86-64 -B i386:x86-64
-endif
-ifeq ($(XEN_TARGET_ARCH),arm64)
-OBJCOPY_MAGIC := -I binary -O elf64-littleaarch64 -B aarch64
-endif
-ifeq ($(XEN_TARGET_ARCH),arm32)
-OBJCOPY_MAGIC := -I binary -O elf32-littlearm -B arm
-endif
-
+NOTE_SYMBOL = "note_depends"
+NOTE_DEPENDS = "const  __section(\".livepatch.depends\") $(NOTE_SYMBOL)"
 CODE_ADDR=$(shell nm --defined $(1) | grep $(2) | awk '{print "0x"$$1}')
 CODE_SZ=$(shell nm --defined -S $(1) | grep $(2) | awk '{ print "0x"$$2}')
 
@@ -38,7 +30,7 @@ uninstall:
 
 .PHONY: clean
 clean::
-	rm -f *.o .*.o.d *.livepatch config.h
+	rm -f *.o .*.o.d *.livepatch config.h livepatch_depends.h hello_world_livepatch_depends.h *.bin
 
 #
 # To compute these values we need the binary files: xen-syms
@@ -56,10 +48,10 @@ config.h: xen_hello_world_func.o
 	 echo "#define MINOR_VERSION_ADDR $(MINOR_VERSION_ADDR)"; \
 	 echo "#define OLD_CODE_SZ $(OLD_CODE_SZ)") > $@
 
-xen_hello_world.o: config.h
+xen_hello_world.o: config.h livepatch_depends.h
 
 .PHONY: $(LIVEPATCH)
-$(LIVEPATCH): xen_hello_world_func.o xen_hello_world.o note.o
+$(LIVEPATCH): xen_hello_world_func.o xen_hello_world.o
 	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH) $^
 
 #
@@ -71,40 +63,42 @@ $(LIVEPATCH): xen_hello_world_func.o xen_hello_world.o note.o
 # not be built (it is for EFI builds), and that we do not have
 # the note.o.bin to muck with (as it gets deleted)
 #
-.PHONY: note.o
-note.o:
-	$(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(BASEDIR)/xen-syms $@.bin
-	$(OBJCOPY) $(OBJCOPY_MAGIC) \
-		   --rename-section=.data=.livepatch.depends,alloc,load,readonly,data,contents -S $@.bin $@
-	rm -f $@.bin
+.PHONY: note.bin
+note.bin:
+	$(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(BASEDIR)/xen-syms $@
+
+.PHONY: livepatch_depends.h
+livepatch_depends.h: note.bin
+	$(shell (../../../tools/misc/mkhex $(NOTE_DEPENDS) $^ > $@))
 
 #
 # Extract the build-id of the xen_hello_world.livepatch
 # (which xen_bye_world will depend on).
 #
-.PHONY: hello_world_note.o
-hello_world_note.o: $(LIVEPATCH)
-	$(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(LIVEPATCH) $@.bin
-	$(OBJCOPY) $(OBJCOPY_MAGIC) \
-		   --rename-section=.data=.livepatch.depends,alloc,load,readonly,data,contents -S $@.bin $@
-	rm -f $@.bin
+.PHONY: hello_world_note.bin
+hello_world_note.bin: $(LIVEPATCH)
+	$(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(LIVEPATCH) $@
+
+.PHONY: hello_world_livepatch_depends.h
+hello_world_livepatch_depends.h: hello_world_note.bin
+	$(shell (../../../tools/misc/mkhex $(NOTE_DEPENDS) $^ > $@))
 
-xen_bye_world.o: config.h
+xen_bye_world.o: config.h hello_world_livepatch_depends.h
 
 .PHONY: $(LIVEPATCH_BYE)
-$(LIVEPATCH_BYE): xen_bye_world_func.o xen_bye_world.o hello_world_note.o
+$(LIVEPATCH_BYE): xen_bye_world_func.o xen_bye_world.o
 	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_BYE) $^
 
-xen_replace_world.o: config.h
+xen_replace_world.o: config.h livepatch_depends.h
 
 .PHONY: $(LIVEPATCH_REPLACE)
-$(LIVEPATCH_REPLACE): xen_replace_world_func.o xen_replace_world.o note.o
+$(LIVEPATCH_REPLACE): xen_replace_world_func.o xen_replace_world.o
 	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_REPLACE) $^
 
-xen_nop.o: config.h
+xen_nop.o: config.h livepatch_depends.h
 
 .PHONY: $(LIVEPATCH_NOP)
-$(LIVEPATCH_NOP): xen_nop.o note.o
+$(LIVEPATCH_NOP): xen_nop.o
 	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_NOP) $^
 
 .PHONY: livepatch
diff --git a/xen/test/livepatch/xen_bye_world.c b/xen/test/livepatch/xen_bye_world.c
index 2700f0eedd..935e76ca8b 100644
--- a/xen/test/livepatch/xen_bye_world.c
+++ b/xen/test/livepatch/xen_bye_world.c
@@ -10,6 +10,7 @@
 #include <xen/livepatch.h>
 
 #include <public/sysctl.h>
+#include "hello_world_livepatch_depends.h"
 
 static const char bye_world_patch_this_fnc[] = "xen_extra_version";
 extern const char *xen_bye_world(void);
diff --git a/xen/test/livepatch/xen_hello_world.c b/xen/test/livepatch/xen_hello_world.c
index 02f3f85dc0..988a3b14f4 100644
--- a/xen/test/livepatch/xen_hello_world.c
+++ b/xen/test/livepatch/xen_hello_world.c
@@ -11,6 +11,7 @@
 #include <xen/livepatch_payload.h>
 
 #include <public/sysctl.h>
+#include "livepatch_depends.h"
 
 static const char hello_world_patch_this_fnc[] = "xen_extra_version";
 extern const char *xen_hello_world(void);
diff --git a/xen/test/livepatch/xen_nop.c b/xen/test/livepatch/xen_nop.c
index a224b7c670..8d0c8f5097 100644
--- a/xen/test/livepatch/xen_nop.c
+++ b/xen/test/livepatch/xen_nop.c
@@ -7,6 +7,7 @@
 #include <xen/types.h>
 
 #include <public/sysctl.h>
+#include "livepatch_depends.h"
 
 /*
  * All of the .new_size and .old_addr are based on assumptions that the
diff --git a/xen/test/livepatch/xen_replace_world.c b/xen/test/livepatch/xen_replace_world.c
index 78a8f528b3..a653cc4268 100644
--- a/xen/test/livepatch/xen_replace_world.c
+++ b/xen/test/livepatch/xen_replace_world.c
@@ -9,6 +9,7 @@
 #include <xen/livepatch.h>
 
 #include <public/sysctl.h>
+#include "livepatch_depends.h"
 
 static const char xen_replace_world_name[] = "xen_extra_version";
 extern const char *xen_replace_world(void);
-- 
2.13.3


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