From patchwork Wed Sep 20 22:31:44 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Konrad Rzeszutek Wilk X-Patchwork-Id: 9962717 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4C70B601D5 for ; Wed, 20 Sep 2017 22:34:52 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3E59A29258 for ; Wed, 20 Sep 2017 22:34:52 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 331BF29272; Wed, 20 Sep 2017 22:34:52 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.6 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RCVD_IN_SORBS_SPAM,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 25F2D29258 for ; Wed, 20 Sep 2017 22:34:51 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dunXS-0007H4-Qr; Wed, 20 Sep 2017 22:32:14 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dunXR-0007G1-OT for xen-devel@lists.xenproject.org; Wed, 20 Sep 2017 22:32:13 +0000 Received: from [85.158.139.211] by server-1.bemta-5.messagelabs.com id 2D/A0-02023-C6CE2C95; Wed, 20 Sep 2017 22:32:12 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrHIsWRWlGSWpSXmKPExsVyMfTGQd2cN4c iDa7s4LD4vmUykwOjx+EPV1gCGKNYM/OS8isSWDMWbzzFVPDRteLLl9wGxlabLkYuDiGB6YwS xy5MYQFxWAQ+sEj8W3OcEcSREJjGKrF9yi3mLkZOICdL4urJxywQdprEu3VPoewKiXVnOplAb CEBJYktkx8zQow9xCTxYudlNpCEsICexORvtxlBbDYBfYmna68BDeUAanaT+HSNC6ReRKCdUW L2lV+sIDXMAtUSey5uZITozZHYunoqWD2LgKrE631CIGFeAWuJG/efQd0mLzGxdxpYOaeAjcT +T53MEPdYSzzZepptAqPwAkaGVYzqxalFZalFupZ6SUWZ6RkluYmZObqGBqZ6uanFxYnpqTmJ ScV6yfm5mxiB4ckABDsY17Y6H2KU5GBSEuW1fnkoUogvKT+lMiOxOCO+qDQntfgQowwHh5IEb 8JroJxgUWp6akVaZg4wUmDSEhw8SiK8C14BpXmLCxJzizPTIVKnGC05Lty59IeJ48CeW0Cy4+ bdP0xCLHn5ealS4rxfQRoEQBoySvPgxsGi+RKjrJQwLyPQgUI8BalFuZklqPKvGMU5GJWEebt AruLJzCuB2/oK6CAmoIOyNxwAOagkESEl1cCYk6TVcbRXvfZv3nXtPW1XIiJ0e00nu13nMbFa 8fHRvWMTVDapaLqwr3ey3zv5y3/uv/n/GZ411W6U28IXxBp8U+pZHVvH+c2Nz01blIQTek+vW LnloOuGz1uO5TvHTGtS396//seE3zydN8uy+/L9U9rs4njjDaYkNJ7g2fhfx+7xPvFNIv+VWI ozEg21mIuKEwEub5ih4QIAAA== X-Env-Sender: ketuzsezr@gmail.com X-Msg-Ref: server-13.tower-206.messagelabs.com!1505946731!97016128!1 X-Originating-IP: [209.85.216.193] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 9.4.45; banners=-,-,- X-VirusChecked: Checked Received: (qmail 25497 invoked from network); 20 Sep 2017 22:32:11 -0000 Received: from mail-qt0-f193.google.com (HELO mail-qt0-f193.google.com) (209.85.216.193) by server-13.tower-206.messagelabs.com with AES128-GCM-SHA256 encrypted SMTP; 20 Sep 2017 22:32:11 -0000 Received: by mail-qt0-f193.google.com with SMTP id q8so2677350qtb.1 for ; Wed, 20 Sep 2017 15:32:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=VodlQoNmgxYbYFsWxEHcW2Vy1KLwZvlNGP6hMMQzvPo=; b=czeqINVd6vgGMfrVnQ2Et93kwRYQgnkoli11nEIY9/hmUYu+2KzIYmtErk42k2uBS7 /qmxrnXwiWpRgM/A1Q/OT+NREboNHCCLDRnwMKFzXvxC2ltUbW5nmxZsn7FP+VZ05FmL 8djZMA2M7wLQrnXWIyWH2HDTsTMpg44DylSFflF5kxsQjH4OL+OFnVZK9w15u2e8Ipk7 +T3EtsCmiTej1t98GJoqNaDNR8ueva+KjK7adKMH6QEuBoqNQEM+3td1/sRkh2Pps3jj v6YXU8vc9C08xoq1itQBQDUG3QL+ID0d7IMYby9Pv4sxhhmFH3getJR/c4Hf/z/EITve 3oWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references; bh=VodlQoNmgxYbYFsWxEHcW2Vy1KLwZvlNGP6hMMQzvPo=; b=FZIDqJgTc1Ovl8VlUPFpyg0aok6GDK8qnFg1xMO5PQY7bZirrkFPYND0tamdcV5B2I J3R1T64+wO4JWVweAJcanfFRT2aP1pFXTSORjd0dBjTMgCDmh2iz4eAk1lqg+RiXUkdr mjbbbQqtkkUgn4c8DBxBAXF8sBLads2qydQrRX3ApgGFKcjHvRDNZI+kDeRk5xJiQOdc sSR/K0NRVmW13wmFkOff0xrD6w7r+guYDmgTJQb2iaVdKlCrW0QQupULVbb1Pydu2aTC 4H5ELpJ/1rNNKguSri6xKmEUXfltQwWeDYuFrFBmIMG8zhFk42tpadqG+bKKnpDCt4N2 bJIw== X-Gm-Message-State: AHPjjUh9zfGvKv9cuDBtI3/NVtFCrZRZy4wTTsKgGFPuxSSXeYoSRrrS kkgUIYwlI4umuZHEWec5XETeMw== X-Google-Smtp-Source: AOwi7QDhspjbNYL7AfZci/VthPTosBQ+MVfAIt30hjPtP7hifokqzeyF/upyRT23s8yLz4QtFf47pQ== X-Received: by 10.237.35.151 with SMTP id j23mr402609qtc.96.1505946730593; Wed, 20 Sep 2017 15:32:10 -0700 (PDT) Received: from localhost.localdomain (209-6-200-48.s4398.c3-0.smr-ubr2.sbo-smr.ma.cable.rcncustomer.com. [209.6.200.48]) by smtp.gmail.com with ESMTPSA id z192sm2012656qka.91.2017.09.20.15.32.09 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 20 Sep 2017 15:32:10 -0700 (PDT) From: Konrad Rzeszutek Wilk X-Google-Original-From: Konrad Rzeszutek Wilk To: xen-devel@lists.xenproject.org, ross.lagerwall@citrix.com, konrad.wilk@oracle.com, julien.grall@arm.com, sstabellini@kernel.org Date: Wed, 20 Sep 2017 18:31:44 -0400 Message-Id: <20170920223148.13137-8-konrad.wilk@oracle.com> X-Mailer: git-send-email 2.13.3 In-Reply-To: <20170920223148.13137-1-konrad.wilk@oracle.com> References: <20170920223148.13137-1-konrad.wilk@oracle.com> Cc: andrew.cooper3@citrix.com, Ian Jackson , Wei Liu , jbeulich@suse.com Subject: [Xen-devel] [PATCH v4 07/11] livepatch/x86/arm[32, 64]: Force .livepatch.depends section to be uint32_t aligned. X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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 Acked-by: Ross Lagerwall --- Cc: Jan Beulich Cc: Andrew Cooper Cc: Ian Jackson Cc: Wei Liu 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 #include +#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 #include +#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 #include +#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 #include +#include "livepatch_depends.h" static const char xen_replace_world_name[] = "xen_extra_version"; extern const char *xen_replace_world(void);