From patchwork Wed Jul 26 19:47:54 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: 9865735 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 D3408603F9 for ; Wed, 26 Jul 2017 19:51:10 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C5E19281E1 for ; Wed, 26 Jul 2017 19:51:10 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BAC4A287A3; Wed, 26 Jul 2017 19:51:10 +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 983002866C for ; Wed, 26 Jul 2017 19:51:09 +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 1daSIa-0008Uz-Lv; Wed, 26 Jul 2017 19:48:48 +0000 Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1daSIZ-0008Uj-1m for xen-devel@lists.xenproject.org; Wed, 26 Jul 2017 19:48:47 +0000 Received: from [193.109.254.147] by server-4.bemta-6.messagelabs.com id 4F/46-02962-E12F8795; Wed, 26 Jul 2017 19:48:46 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrLIsWRWlGSWpSXmKPExsVyMfTGYV3ZTxW RBm1nlS2+b5nM5MDocfjDFZYAxijWzLyk/IoE1ozG+fvYC97bVyxbu5SpgbHBuIuRi0NIYDqj xIcX+5hAHBaBDywSzY0bwBwJgWmsEvM797F1MXICOXESs5ZMYIWwqyT2vN/OAmILCShJbJn8m BHCfs4kcWCSPIgtLKAnMfnbbbA4m4C+xNO115hBbBGBPImu/Q1gNrNAkcTze3/Yuxg5gOpzJc 6/zgAJswioSqybvhislVfATOL//yMsEGvlJSb2TgOLcwqYS7x/9owJYq2ZRPe9ZsYJjIILGBl WMWoUpxaVpRbpGhrrJRVlpmeU5CZm5ugaGpjp5aYWFyemp+YkJhXrJefnbmIEBhwDEOxg/LIs 4BCjJAeTkijvJNOKSCG+pPyUyozE4oz4otKc1OJDjDIcHEoSvG0fgHKCRanpqRVpmTnA0IdJS 3DwKInw7gBJ8xYXJOYWZ6ZDpE4x2nNcubLuCxPHlAPbgeSrCf+/MXE0ff/4nUmIJS8/L1VKnL cYpE0ApC2jNA9uKCxWLzHKSgnzMgKdKcRTkFqUm1mCKv+KUZyDUUmYt/U90BSezLwSuN2vgM5 iAjprzoxSkLNKEhFSUg2MNTbmvDdeO3IsXZrFo2/8x8WEIU5hTfO6+xdnJyryPb88Mbfkp4T7 uk7FjleBE+MDj7JfSl/uudgm7VFkiIqsff7WRdW+ymb27otvstlavVSyODzHVEDm2k45pth7Q W82zbyy7OQTTx72JYtyZgokXjuaEurJ6iX7YoJ6sZOw8/S9O/jLODcpsRRnJBpqMRcVJwIAbb 70eNACAAA= X-Env-Sender: ketuzsezr@gmail.com X-Msg-Ref: server-13.tower-27.messagelabs.com!1501098524!99495191!1 X-Originating-IP: [209.85.216.195] X-SpamReason: No, hits=0.5 required=7.0 tests=BODY_RANDOM_LONG X-StarScan-Received: X-StarScan-Version: 9.4.25; banners=-,-,- X-VirusChecked: Checked Received: (qmail 31942 invoked from network); 26 Jul 2017 19:48:45 -0000 Received: from mail-qt0-f195.google.com (HELO mail-qt0-f195.google.com) (209.85.216.195) by server-13.tower-27.messagelabs.com with AES128-GCM-SHA256 encrypted SMTP; 26 Jul 2017 19:48:45 -0000 Received: by mail-qt0-f195.google.com with SMTP id p3so8081417qtg.5 for ; Wed, 26 Jul 2017 12:48:45 -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=VxftRgF7sTxmPY/CaH9Fo4zfriqhOb9WBXTXASlfYVc=; b=M7/yAt6pqHo+/FOfxgM600erqwB309n1w7FMGYrLAjD9YteOu5beA7McP+m3GdedDI CuwIqQYqgnOPfdmOSESE/tsZRfe4zBDwRKn9jmZQcX1IrPRgwYZ/GHh86R3MQVeoqsS7 XkfFWPrrgbmAoHQ+k7C9b+42T8LJt9sf9dqzSwFqkpwKteIPTuQ3rPS9CCWiMBtIqYYs LF8E4tV0MuFGm89qyZfaoITz5iZcpQZdlTBSklhyFjHSIVKbXnSxvFKJR3bK3TLyy4jF XgfT6CLr49sbj6M60aP4Fc9UISL8QbHdnsK7va+dTZCVsx+55cnll/OnV8b16N0MJlA8 /0mw== 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=VxftRgF7sTxmPY/CaH9Fo4zfriqhOb9WBXTXASlfYVc=; b=F/KsYCV/0oG+ehFcTq6jUumydkptZQ8xzNswZDyFPUIdFRvRqNLQmzUihkhkeByTd7 05AE7v5eIrPSxdBzMM0G7BQBKlVmIrkTNgvldL0WuIvEHGOr8CdyrqpmPOptJFgMycTR jIyrKAB1BQPjREamfRVrSAgDl0LFG4R47Cfxn8v6gFHWe/EUONFqo2RURp3XCJB7tOWl mY0fOTZgCOFkliDuTRrjheW8fUsQ0i0Y7LoRSPOkMVW3IQtc2Gv2XpX9oD2laHLhlWMR b7JWZTWgvSfM61rDyuaXaMYfWIhO15HsV7E7kOEmnRQEpgQhO4Hjx9MEeFRK8AX74TDL nKCw== X-Gm-Message-State: AIVw110sTVfpkuUoaUwDpWscrRgglRdGbx+I4t68FnxPIM/JFfOMjwIO 1AytH5rkO0+lapSx X-Received: by 10.237.53.226 with SMTP id d31mr2763538qte.266.1501098523927; Wed, 26 Jul 2017 12:48:43 -0700 (PDT) Received: from dhcp-amer-vpn-adc-anyconnect-10-154-174-211.vpn.oracle.com (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 m22sm13305504qtm.15.2017.07.26.12.48.42 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 26 Jul 2017 12:48:42 -0700 (PDT) From: Konrad Rzeszutek Wilk To: xen-devel@lists.xenproject.org, julien.grall@arm.com, sstabellini@kernel.org, andrew.cooper3@citrix.com Date: Wed, 26 Jul 2017 15:47:54 -0400 Message-Id: <20170726194756.20265-4-konrad@kernel.org> X-Mailer: git-send-email 2.13.3 In-Reply-To: <20170726194756.20265-1-konrad@kernel.org> References: <20170726194756.20265-1-konrad@kernel.org> Cc: Konrad Rzeszutek Wilk , jbeulich@suse.com Subject: [Xen-devel] [PATCH v2 3/5] xen/livepatch/ARM32: Don't load and crash on livepatches loaded with wrong alignment. 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 This issue was observed on ARM32 with a cross compiler for the livepatches. Mainly the livepatches .data section size was not padded to the section alignment: ARM32 native: Contents of section .rodata: 0000 68695f66 756e6300 63686563 6b5f666e hi_func.check_fn 0010 63000000 78656e5f 65787472 615f7665 c...xen_extra_ve 0020 7273696f 6e000000 rsion... ARM32 cross compiler: Contents of section .rodata: 0000 68695f66 756e6300 63686563 6b5f666e hi_func.check_fn 0010 63000000 78656e5f 65787472 615f7665 c...xen_extra_ve 0020 7273696f 6e00 rsion. And when we loaded it the next section would be put right behind it: native: (XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .text at 00a02000 (XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .rodata at 00a04024 (XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .altinstructions at 00a0404c cross compiler: (XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .text at 00a02000 (XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .rodata at 00a04024 (XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .altinstructions at 00a0404a (See 4a vs 4c) native readelf: [ 4] .rodata PROGBITS 00000000 000164 000028 00 A 0 0 4 [ 5] .altinstructions PROGBITS 00000000 00018c 00000c 00 A 0 0 1 cross compiler readelf --sections: [ 4] .rodata PROGBITS 00000000 000164 000026 00 A 0 0 4 [ 5] .altinstructions PROGBITS 00000000 00018a 00000c 00 A 0 0 1 And as can be seen the .altinstructions have alignment of 1 which from 'man elf' is: "Values of zero and one mean no alignment is required." which means we can ignore it. See patch titled "alternative/x86/arm32: Align altinstructions (and altinstr_replacement) sections." which fixes the .altinstructions to have the proper alignment. Ignoring this will result in a crash as when we started processing ".rel.altinstructions" for ".altinstructions" with a cross-compiled payload we would end up poking in an section that was not aligned properly in memory and crash. This allows us on ARM32 to error out with: livepatch: xen_hello_world: dest=00a0404a (.altinstructions) is not aligned properly! Furthermore we also observe that the alignment is not correct for other sections (when cross compiling) as such we add the check in various other places which allows us to get. livepatch: xen_bye_world: .livepatch.depends alignment is wrong! instead of a crash. (See patch "xen/livepatch/x86/arm32: Force livepatch_depends to be uint32_t aligned" which fixes the .livepatch.depends alignment) Signed-off-by: Konrad Rzeszutek Wilk --- v1: Initial patch v2: Redo the commit to include the commits which fix the alignment issues. Also mention the need in the docs --- docs/misc/livepatch.markdown | 6 +++++- xen/arch/arm/arm32/livepatch.c | 15 +++++++++++++++ xen/arch/arm/arm64/livepatch.c | 6 ++++++ xen/arch/x86/livepatch.c | 6 ++++++ xen/common/livepatch.c | 14 +++++++++++++- xen/include/xen/livepatch.h | 1 + 6 files changed, 46 insertions(+), 2 deletions(-) diff --git a/docs/misc/livepatch.markdown b/docs/misc/livepatch.markdown index 54a6b850cb..c2c4acd3d3 100644 --- a/docs/misc/livepatch.markdown +++ b/docs/misc/livepatch.markdown @@ -279,6 +279,10 @@ It may also have some architecture-specific sections. For example: * Exception tables. * Relocations for each of these sections. +Note that on ARM 32 the sections SHOULD be four byte aligned. Otherwise +we risk hitting Data Abort exception as un-aligned manipulation of data is +prohibited on ARM 32. + The Xen Live Patch core code loads the payload as a standard ELF binary, relocates it and handles the architecture-specifc sections as needed. This process is much like what the Linux kernel module loader does. @@ -341,7 +345,7 @@ The size of the structure is 64 bytes on 64-bit hypervisors. It will be * `opaque` **MUST** be zero. The size of the `livepatch_func` array is determined from the ELF section -size. +size. On ARM32 this section must by four-byte aligned. When applying the patch the hypervisor iterates over each `livepatch_func` structure and the core code inserts a trampoline at `old_addr` to `new_addr`. diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c index 41378a54ae..d48820f8ba 100644 --- a/xen/arch/arm/arm32/livepatch.c +++ b/xen/arch/arm/arm32/livepatch.c @@ -112,6 +112,12 @@ bool arch_livepatch_symbol_deny(const struct livepatch_elf *elf, return false; } +bool arch_livepatch_verify_alignment(const struct livepatch_elf_sec *sec) +{ + /* Unaligned access on ARM 32 will crash with Data Abort. */ + return (uint32_t)sec->load_addr % sizeof(uint32_t); +}; + static s32 get_addend(unsigned char type, void *dest) { s32 addend = 0; @@ -266,6 +272,15 @@ int arch_livepatch_perform(struct livepatch_elf *elf, elf->name, symndx); return -EINVAL; } + + if ( !use_rela ) + addend = get_addend(type, dest); + else if ( (uint32_t)dest % sizeof(uint32_t) ) + { + dprintk(XENLOG_ERR, LIVEPATCH "%s: dest=%p (%s) is not aligned properly!\n", + elf->name, dest, base->name); + return -EINVAL; + } else if ( !elf->sym[symndx].sym ) { dprintk(XENLOG_ERR, LIVEPATCH "%s: No relative symbol@%u\n", diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c index 2247b925a0..7b36210ccd 100644 --- a/xen/arch/arm/arm64/livepatch.c +++ b/xen/arch/arm/arm64/livepatch.c @@ -86,6 +86,12 @@ bool arch_livepatch_symbol_deny(const struct livepatch_elf *elf, return false; } +bool arch_livepatch_verify_alignment(const struct livepatch_elf_sec *sec) +{ + /* Unaligned access on ARM 64 is OK. */ + return false; +} + enum aarch64_reloc_op { RELOC_OP_NONE, RELOC_OP_ABS, diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c index 406eb910cc..b3cbdac9b7 100644 --- a/xen/arch/x86/livepatch.c +++ b/xen/arch/x86/livepatch.c @@ -148,6 +148,12 @@ bool arch_livepatch_symbol_deny(const struct livepatch_elf *elf, return false; } +bool arch_livepatch_verify_alignment(const struct livepatch_elf_sec *sec) +{ + /* Unaligned access on x86 is fine. */ + return false; +} + int arch_livepatch_perform_rel(struct livepatch_elf *elf, const struct livepatch_elf_sec *base, const struct livepatch_elf_sec *rela) diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index 40ff6b3a27..13d8f25a4b 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -472,6 +472,13 @@ static int check_section(const struct livepatch_elf *elf, return -EINVAL; } + if ( arch_livepatch_verify_alignment(sec) ) + { + dprintk(XENLOG_ERR, LIVEPATCH "%s: %s is not aligned properly!\n", + elf->name, sec->name); + return -EINVAL; + } + return 0; } @@ -501,7 +508,12 @@ static int check_special_sections(const struct livepatch_elf *elf) elf->name, names[i]); return -EINVAL; } - + if ( arch_livepatch_verify_alignment(sec) ) + { + dprintk(XENLOG_ERR, LIVEPATCH "%s: %s is not aligned properly!\n", + elf->name, names[i]); + return -EINVAL; + } if ( test_and_set_bit(i, found) ) { dprintk(XENLOG_ERR, LIVEPATCH "%s: %s was seen more than once!\n", diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h index 98ec01216b..e9bab87f28 100644 --- a/xen/include/xen/livepatch.h +++ b/xen/include/xen/livepatch.h @@ -76,6 +76,7 @@ void arch_livepatch_init(void); #include int arch_livepatch_verify_func(const struct livepatch_func *func); +bool arch_livepatch_verify_alignment(const struct livepatch_elf_sec *sec); static inline unsigned int livepatch_insn_len(const struct livepatch_func *func) {