From patchwork Fri Sep 16 16:38:17 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Konrad Rzeszutek Wilk X-Patchwork-Id: 9336213 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 4605260839 for ; Fri, 16 Sep 2016 16:41:51 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3563E2A038 for ; Fri, 16 Sep 2016 16:41:51 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2A2752A03B; Fri, 16 Sep 2016 16:41:51 +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=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY 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 213EC2A039 for ; Fri, 16 Sep 2016 16:41:50 +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 1bkwAB-0002m7-6b; Fri, 16 Sep 2016 16:38:55 +0000 Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bkwAA-0002k2-J5 for xen-devel@lists.xenproject.org; Fri, 16 Sep 2016 16:38:54 +0000 Received: from [193.109.254.147] by server-2.bemta-6.messagelabs.com id 0F/99-13744-D102CD75; Fri, 16 Sep 2016 16:38:53 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFmpnkeJIrShJLcpLzFFi42KZM10+UFdW4U6 4wdYt/Bbft0xmcmD0OPzhCksAYxRrZl5SfkUCa8beM39ZC86ZVpz78ompgfG7ThcjJ4eQQAeT xOOfZRD2N0aJo39kuhi5gOyNjBI/b9xlhnC6GSXadq5ngagqkjj96xqQzcHBJmAi8WaVI0iNi EAfo8Tto38ZQRxmEOds3wU2kAZhgUiJpevOg9ksAqoSS/7vYAexeQXcJDpvLQYbKiEgJ7Ftyx 5GkKGcAu4SPecyIXa5SSxZv5YJosRYom9WH8sERv4FjAyrGDWKU4vKUot0DS31kooy0zNKchM zc3QNDcz0clOLixPTU3MSk4r1kvNzNzECA4gBCHYw/lgWcIhRkoNJSZS3uPB2uBBfUn5KZUZi cUZ8UWlOavEhRhkODiUJ3q9yd8KFBItS01Mr0jJzgKEMk5bg4FES4V0MkuYtLkjMLc5Mh0idY lSUEuetAUkIgCQySvPg2mDxc4lRVkqYlxHoECGegtSi3MwSVPlXjOIcjErCvI9BpvBk5pXATX 8FtJgJaPHqWbdBFpckIqSkGhgr9r3lsU/sSTtb+n/PxX9Bif/yem5f2nNSXtq0+3lze9d2N7t g7x9OC99cPXvX4GKfqavp2+ty+fvnWX8vbn2/veWN+o97CUu0K7gFnHfPS+q407pWblXU8Zf9 ZzfVuwQm753bpShYcEmP1U/XXuaYV31MQVTKvd4t10sb9pao73k4hTf2/nclluKMREMt5qLiR AAPduvTmgIAAA== X-Env-Sender: konrad@char.us.oracle.com X-Msg-Ref: server-15.tower-27.messagelabs.com!1474043931!6794527!1 X-Originating-IP: [156.151.31.81] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogMTU2LjE1MS4zMS44MSA9PiAyODgzMzk=\n X-StarScan-Received: X-StarScan-Version: 8.84; banners=-,-,- X-VirusChecked: Checked Received: (qmail 29388 invoked from network); 16 Sep 2016 16:38:52 -0000 Received: from userp1040.oracle.com (HELO userp1040.oracle.com) (156.151.31.81) by server-15.tower-27.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 16 Sep 2016 16:38:52 -0000 Received: from aserv0021.oracle.com (aserv0021.oracle.com [141.146.126.233]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id u8GGcZjI030433 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 16 Sep 2016 16:38:35 GMT Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by aserv0021.oracle.com (8.13.8/8.13.8) with ESMTP id u8GGcZ52020861 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 16 Sep 2016 16:38:35 GMT Received: from abhmp0011.oracle.com (abhmp0011.oracle.com [141.146.116.17]) by aserv0121.oracle.com (8.13.8/8.13.8) with ESMTP id u8GGcYlN005055; Fri, 16 Sep 2016 16:38:34 GMT Received: from char.us.oracle.com (/10.137.176.158) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Fri, 16 Sep 2016 09:38:34 -0700 Received: by char.us.oracle.com (Postfix, from userid 1000) id CDD286A0E02; Fri, 16 Sep 2016 12:38:32 -0400 (EDT) From: Konrad Rzeszutek Wilk To: xen-devel@lists.xenproject.org, konrad@kernel.org, ross.lagerwall@citrix.com, julien.grall@arm.com, sstabellini@kernel.org Date: Fri, 16 Sep 2016 12:38:17 -0400 Message-Id: <1474043908-12101-6-git-send-email-konrad.wilk@oracle.com> X-Mailer: git-send-email 2.5.5 In-Reply-To: <1474043908-12101-1-git-send-email-konrad.wilk@oracle.com> References: <1474043908-12101-1-git-send-email-konrad.wilk@oracle.com> X-Source-IP: aserv0021.oracle.com [141.146.126.233] Cc: Andrew Cooper , Jan Beulich , Konrad Rzeszutek Wilk Subject: [Xen-devel] [PATCH v4 05/16] livepatch: ARM/x86: Check displacement of old_addr and new_addr 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 If the distance is too great we are in trouble - as our relocation distance can surely be clipped, or still have a valid width - but cause an overflow of distance. On various architectures the maximum displacement for a unconditional branch/jump varies. ARM32 is +/- 32MB, ARM64 is +/- 128MB while x86 for 32-bit relocations is +/- 2G. Note: On x86 we could use the 64-bit jmpq instruction which would provide much bigger displacement to do a jump, but we would still have issues with the new function not being able to reach any of the old functions (as all the relocations would assume 32-bit displacement). And "furthermore would require an register or memory location to load/store the address to." (From Jan). On ARM the conditional branch supports even a smaller displacement but fortunatly we are not using that. Signed-off-by: Konrad Rzeszutek Wilk --- Cc: Andrew Cooper Cc: Jan Beulich Cc: Stefano Stabellini Cc: Julien Grall v3: New submission. v4: s/arch_livepatch_verify_distance/livepatch_verify_distance/ s/LIVEPATCH_ARCH_RANGE/ARCH_LIVEPATCH_RANGE/ v5: Updated commit description with Jan's comment Ditch the casting of long on calculating offset. --- docs/misc/livepatch.markdown | 14 +++++++++++++- xen/arch/arm/arm64/livepatch.c | 1 + xen/common/livepatch.c | 4 ++++ xen/include/asm-arm/livepatch.h | 11 +++++++++++ xen/include/asm-x86/livepatch.h | 3 +++ xen/include/xen/livepatch.h | 19 +++++++++++++++++-- 6 files changed, 49 insertions(+), 3 deletions(-) diff --git a/docs/misc/livepatch.markdown b/docs/misc/livepatch.markdown index 9e72897..5baaa0a 100644 --- a/docs/misc/livepatch.markdown +++ b/docs/misc/livepatch.markdown @@ -1100,7 +1100,7 @@ and no .data or .bss sections. The hypervisor should verify that the in-place patching would fit within the code or data. -### Trampoline (e9 opcode) +### Trampoline (e9 opcode), x86 The e9 opcode used for jmpq uses a 32-bit signed displacement. That means we are limited to up to 2GB of virtual address to place the new code @@ -1134,3 +1134,15 @@ that in the hypervisor is advised. The tool for generating payloads currently does perform a compile-time check to ensure that the function to be replaced is large enough. +The hypervisor also checks the displacement during loading of the payload. + +#### Trampoline (ea opcode), ARM + +The 0xea000000 instruction (with proper offset) is used for an unconditional +branch to the new code. This means we are limited on ARM32 to +/- 32MB +displacement and on ARM64 to +/- 128MB displacement. + +The new code is placed in the 8M - 10M virtual address space while the +Xen code is in 2M - 4M. That gives us enough space. + +The hypervisor also checks the displacement during loading of the payload. diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c index 49eb69b..7d593b2 100644 --- a/xen/arch/arm/arm64/livepatch.c +++ b/xen/arch/arm/arm64/livepatch.c @@ -40,6 +40,7 @@ void arch_livepatch_apply_jmp(struct livepatch_func *func) else insn = aarch64_insn_gen_nop(); + /* Verified in livepatch_verify_distance. */ ASSERT(insn != AARCH64_BREAK_FAULT); new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text; diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index a4ce8c7..38260b9 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -540,6 +540,10 @@ static int prepare_payload(struct payload *payload, rc = resolve_old_address(f, elf); if ( rc ) return rc; + + rc = livepatch_verify_distance(f); + if ( rc ) + return rc; } sec = livepatch_elf_sec_by_name(elf, ".livepatch.hooks.load"); diff --git a/xen/include/asm-arm/livepatch.h b/xen/include/asm-arm/livepatch.h index 929c7d9..482d74f 100644 --- a/xen/include/asm-arm/livepatch.h +++ b/xen/include/asm-arm/livepatch.h @@ -6,6 +6,8 @@ #ifndef __XEN_ARM_LIVEPATCH_H__ #define __XEN_ARM_LIVEPATCH_H__ +#include /* For SZ_* macros. */ + /* On ARM32,64 instructions are always 4 bytes long. */ #define ARCH_PATCH_INSN_SIZE 4 @@ -15,6 +17,15 @@ */ extern void *vmap_of_xen_text; +/* These ranges are only for unconditional branches. */ +#ifdef CONFIG_ARM_32 +/* ARM32: A4.3 IN ARM DDI 0406C.j - we are using only ARM instructions in Xen.*/ +#define ARCH_LIVEPATCH_RANGE SZ_32M +#else +/* ARM64: C1.3.2 in ARM DDI 0487A.j */ +#define ARCH_LIVEPATCH_RANGE SZ_128M +#endif + #endif /* __XEN_ARM_LIVEPATCH_H__ */ /* diff --git a/xen/include/asm-x86/livepatch.h b/xen/include/asm-x86/livepatch.h index 5e04aa1..7dfc2e7 100644 --- a/xen/include/asm-x86/livepatch.h +++ b/xen/include/asm-x86/livepatch.h @@ -6,7 +6,10 @@ #ifndef __XEN_X86_LIVEPATCH_H__ #define __XEN_X86_LIVEPATCH_H__ +#include /* For SZ_* macros. */ + #define ARCH_PATCH_INSN_SIZE 5 +#define ARCH_LIVEPATCH_RANGE SZ_2G #endif /* __XEN_X86_LIVEPATCH_H__ */ diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h index b714fbc..6ea92b5 100644 --- a/xen/include/xen/livepatch.h +++ b/xen/include/xen/livepatch.h @@ -12,6 +12,7 @@ struct livepatch_elf_sym; struct xen_sysctl_livepatch_op; #include +#include /* For -ENOSYS or -EOVERFLOW */ #ifdef CONFIG_LIVEPATCH /* @@ -68,7 +69,7 @@ int arch_livepatch_secure(const void *va, unsigned int pages, enum va_type types void arch_livepatch_init(void); #include /* For struct livepatch_func. */ -#include /* For ARCH_PATCH_INSN_SIZE. */ +#include /* For ARCH_[PATCH_INSN_SIZE, LIVEPATCH_RANGE]. */ int arch_livepatch_verify_func(const struct livepatch_func *func); static inline size_t livepatch_insn_len(const struct livepatch_func *func) @@ -78,6 +79,21 @@ static inline size_t livepatch_insn_len(const struct livepatch_func *func) return ARCH_PATCH_INSN_SIZE; } + +static inline int livepatch_verify_distance(const struct livepatch_func *func) +{ + long offset; + long range = (long)ARCH_LIVEPATCH_RANGE; + + if ( !func->new_addr ) /* Ignore NOPs. */ + return 0; + + offset = func->old_addr - func->new_addr; + if ( offset < -range || offset >= range ) + return -EOVERFLOW; + + return 0; +} /* * These functions are called around the critical region patching live code, * for an architecture to take make appropratie global state adjustments. @@ -102,7 +118,6 @@ void arch_livepatch_unmask(void); #define init_or_livepatch_data __initdata #define init_or_livepatch __init -#include /* For -ENOSYS */ static inline int livepatch_op(struct xen_sysctl_livepatch_op *op) { return -ENOSYS;