From patchwork Wed Sep 21 16:57:08 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: 9343953 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 73A4B60B16 for ; Wed, 21 Sep 2016 16:59:54 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7456F2A80B for ; Wed, 21 Sep 2016 16:59:54 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 67B7B2A80D; Wed, 21 Sep 2016 16:59:54 +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 9784A2A80B for ; Wed, 21 Sep 2016 16:59:53 +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 1bmkpv-0000H9-Jm; Wed, 21 Sep 2016 16:57:31 +0000 Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bmkpv-0000GP-3e for xen-devel@lists.xenproject.org; Wed, 21 Sep 2016 16:57:31 +0000 Received: from [85.158.143.35] by server-4.bemta-6.messagelabs.com id C0/E5-29421-AFBB2E75; Wed, 21 Sep 2016 16:57:30 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrFLMWRWlGSWpSXmKPExsXSO6nOVffn7kf hBose61h83zKZyYHR4/CHKywBjFGsmXlJ+RUJrBnf781kL5gWXLHo5BTmBsaTzl2MXBxCApOZ JNatmc4M4fxmlPjz8CoLhLORUaJr1huozCxGiXv3JrN2MXJwsAmYSLxZ5djFyMkhIhAh0Tf1N SOIzSxQJvFw8RNWEFtYwFHi595GJhCbRUBVom/KfGYQm1fATaJxwxdGkDESAvISz27Xg4Q5Bd wlPr7bzgISFgIqWfAjFSQsIWAs0f72ItsERr4FjAyrGDWKU4vKUot0DY30kooy0zNKchMzc3Q NDcz0clOLixPTU3MSk4r1kvNzNzECw4QBCHYwXt4YcIhRkoNJSZS3e8ujcCG+pPyUyozE4oz4 otKc1OJDjDIcHEoSvP93AeUEi1LTUyvSMnOAAQuTluDgURLh9QVJ8xYXJOYWZ6ZDpE4xKkqJ8 0YBw1xIACSRUZoH1waLkkuMslLCvIxAhwjxFKQW5WaWoMq/YhTnYFQS5tUFmcKTmVcCN/0V0G ImoMVbfj4AWVySiJCSamBkF1FauZ5T/vOtHPFPEXVXZib0/q449yhDwjJj3ya9qTfn5ezc2fn kscyemVseCS7nvrYj/6qQoIv6Z8uP+nybPxQ+N6ipebf6xplTYru3suvJmZ3YdLa9ZMPbD2wR kjwxnoLWL3WvLHc/ObG+wmBdOXtbs/aGENkpaz9d7qkvU9qTwPCqYZaMEktxRqKhFnNRcSIAe 2JG840CAAA= X-Env-Sender: konrad.wilk@oracle.com X-Msg-Ref: server-10.tower-21.messagelabs.com!1474477047!27763371!1 X-Originating-IP: [141.146.126.69] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogMTQxLjE0Ni4xMjYuNjkgPT4gMjc3MjE4\n X-StarScan-Received: X-StarScan-Version: 8.84; banners=-,-,- X-VirusChecked: Checked Received: (qmail 13227 invoked from network); 21 Sep 2016 16:57:29 -0000 Received: from aserp1040.oracle.com (HELO aserp1040.oracle.com) (141.146.126.69) by server-10.tower-21.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 21 Sep 2016 16:57:29 -0000 Received: from aserv0022.oracle.com (aserv0022.oracle.com [141.146.126.234]) by aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id u8LGvNMk003198 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 21 Sep 2016 16:57:23 GMT Received: from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75]) by aserv0022.oracle.com (8.14.4/8.14.4) with ESMTP id u8LGvN3W010907 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 21 Sep 2016 16:57:23 GMT Received: from abhmp0007.oracle.com (abhmp0007.oracle.com [141.146.116.13]) by userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id u8LGvM9o016862; Wed, 21 Sep 2016 16:57:22 GMT Received: from localhost.localdomain.localdomain (/172.58.216.202) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 21 Sep 2016 09:57:22 -0700 From: Konrad Rzeszutek Wilk To: konrad@kernel.org, xen-devel@lists.xenproject.org, ross.lagerwall@citrix.com Date: Wed, 21 Sep 2016 12:57:08 -0400 Message-Id: <1474477030-10722-4-git-send-email-konrad.wilk@oracle.com> X-Mailer: git-send-email 2.4.11 In-Reply-To: <1474477030-10722-1-git-send-email-konrad.wilk@oracle.com> References: <1474477030-10722-1-git-send-email-konrad.wilk@oracle.com> X-Source-IP: aserv0022.oracle.com [141.146.126.234] Cc: andrew.cooper3@citrix.com, Jan Beulich , Konrad Rzeszutek Wilk Subject: [Xen-devel] [PATCH v7 3/5] livepatch: NOP if func->new_addr is zero. 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 The NOP functionality will NOP any of the code at the 'old_addr' or at 'name' if the 'new_addr' is zero. The purpose of this is to NOP out calls, such as: e8 <4-bytes-offset> (5 byte insn), or on ARM a 4 byte insn for branching. We need the EIP of where we need to the NOP, and that can be provided via the `old_addr` or `name`. If the `old_addr` is provided we will NOP 'new_size' amount of bytes at that location. The amount is up to 31 instructions if desired (which is the size of the opaque member). If there is a need to NOP more then: a) more 'struct livepatch_func' structures need to be present, b) we have to implement a variable size buffer (in the future), or c) first byte an unconditional branch skipping the to be disabled code (of course provided there are no branch targets in the middle). While at it, also unify the code on x86 patching so it is a bit simpler (instead of two seperate writes just make it one memcpy). And introduce a general livepatch_insn_len inline function that would depend on platform specific instruction size (for a unconditional branch). As such we also rename the PATCH_INSN_SIZE to ARCH_PATCH_INSN_SIZE. Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: Jan Beulich --- Cc: Konrad Rzeszutek Wilk Cc: Ross Lagerwall Cc: Jan Beulich Cc: Andrew Cooper v3: First submission v4: Fix description - e9 -> e8 Remove the restriction of only doing 5 or 4 bytes. Redo the patching code to deal with variable size of new_size. Expand the amount of bytes we can NOP. Move the PATCH_INSN_SIZE definition in platform specific headers Move the get_len to livepatch_get_insn_len inline function. v5: s/PATCH_INSN_SIZE/ARCH_PATCH_INSN_SIZE/ s/arch_livepatch_insn_len/livepatch_insn_len/ s/size_t len/unsigned int len/ Add in commit description the c) mechanism (insert an unconditional branch). v6: Expand in the documentation about the old_addr being at least 5. v7: Added the extra entry in the MAINTAINERS file. - Expanded on the livepatch.markdown the description. - Used add_nops(insn, .. instead of add_nops(&insn, ..) --- MAINTAINERS | 1 + docs/misc/livepatch.markdown | 22 ++++++++++++++---- xen/arch/x86/alternative.c | 2 +- xen/arch/x86/livepatch.c | 48 +++++++++++++++++++++++++++------------ xen/common/livepatch.c | 3 ++- xen/include/asm-x86/alternative.h | 1 + xen/include/asm-x86/livepatch.h | 21 +++++++++++++++++ xen/include/xen/livepatch.h | 10 ++++++++ 8 files changed, 86 insertions(+), 22 deletions(-) create mode 100644 xen/include/asm-x86/livepatch.h diff --git a/MAINTAINERS b/MAINTAINERS index 97720a8..9b30600 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -270,6 +270,7 @@ F: docs/misc/livepatch.markdown F: tools/misc/xen-livepatch.c F: xen/arch/*/livepatch* F: xen/common/livepatch* +F: xen/include/asm-*/livepatch.h F: xen/include/xen/livepatch* MACHINE CHECK (MCA) & RAS diff --git a/docs/misc/livepatch.markdown b/docs/misc/livepatch.markdown index 81f4fc9..37a0860 100644 --- a/docs/misc/livepatch.markdown +++ b/docs/misc/livepatch.markdown @@ -318,13 +318,24 @@ The size of the structure is 64 bytes on 64-bit hypervisors. It will be payload generation time if hypervisor function address is known. If unknown, the value *MUST* be zero and the hypervisor will attempt to resolve the address. -* `new_addr` is the address of the function that is replacing the old - function. The address is filled in during relocation. The value **MUST** be - the address of the new function in the file. +* `new_addr` can either have a non-zero value or be zero. + * If there is a non-zero value, then it is the address of the function that is + replacing the old function and the address is recomputed during relocation. + The value **MUST** be the address of the new function in the payload file. -* `old_size` and `new_size` contain the sizes of the respective functions in bytes. + * If the value is zero, then we NOPing out at the `old_addr` location + `new_size` bytes. + +* `old_size` contains the sizes of the respective `old_addr` function in bytes. The value of `old_size` **MUST** not be zero. +* `new_size` depends on what `new_addr` contains: + * If `new_addr` contains an non-zero value, then `new_size` has the size of + the new function (which will replace the one at `old_addr`) in bytes. + * If the value of `new_addr` is zero then `new_size` determines how many + instruction bytes to NOP (up to opaque size modulo smallest platform + instruction - 1 byte x86 and 4 bytes on ARM). + * `version` is to be one. * `opaque` **MUST** be zero. @@ -1087,7 +1098,8 @@ limit that calls the next trampoline. Please note there is a small limitation for trampolines in function entries: The target function (+ trailing padding) must be able to accomodate the trampoline. On x86 with +-2 GB relative jumps, -this means 5 bytes are required. +this means 5 bytes are required which means that `old_size` **MUST** be +at least five bytes if patching in trampoline. Depending on compiler settings, there are several functions in Xen that are smaller (without inter-function padding). diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c index 05e3eb8..6eaa10f 100644 --- a/xen/arch/x86/alternative.c +++ b/xen/arch/x86/alternative.c @@ -101,7 +101,7 @@ static void __init arch_init_ideal_nops(void) } /* Use this to add nops to a buffer, then text_poke the whole buffer. */ -static void init_or_livepatch add_nops(void *insns, unsigned int len) +void init_or_livepatch add_nops(void *insns, unsigned int len) { while ( len > 0 ) { diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c index 56da154..d5e7174 100644 --- a/xen/arch/x86/livepatch.c +++ b/xen/arch/x86/livepatch.c @@ -12,8 +12,7 @@ #include #include - -#define PATCH_INSN_SIZE 5 +#include int arch_livepatch_quiesce(void) { @@ -31,11 +30,17 @@ void arch_livepatch_revive(void) int arch_livepatch_verify_func(const struct livepatch_func *func) { - /* No NOP patching yet. */ - if ( !func->new_size ) - return -EOPNOTSUPP; + /* If NOPing.. */ + if ( !func->new_addr ) + { + /* Only do up to maximum amount we can put in the ->opaque. */ + if ( func->new_size > sizeof(func->opaque) ) + return -EOPNOTSUPP; - if ( func->old_size < PATCH_INSN_SIZE ) + if ( func->old_size < func->new_size ) + return -EINVAL; + } + else if ( func->old_size < ARCH_PATCH_INSN_SIZE ) return -EINVAL; return 0; @@ -43,23 +48,36 @@ int arch_livepatch_verify_func(const struct livepatch_func *func) void arch_livepatch_apply_jmp(struct livepatch_func *func) { - int32_t val; uint8_t *old_ptr; - - BUILD_BUG_ON(PATCH_INSN_SIZE > sizeof(func->opaque)); - BUILD_BUG_ON(PATCH_INSN_SIZE != (1 + sizeof(val))); + uint8_t insn[sizeof(func->opaque)]; + unsigned int len; old_ptr = func->old_addr; - memcpy(func->opaque, old_ptr, PATCH_INSN_SIZE); + len = livepatch_insn_len(func); + if ( !len ) + return; + + memcpy(func->opaque, old_ptr, len); + if ( func->new_addr ) + { + int32_t val; + + BUILD_BUG_ON(ARCH_PATCH_INSN_SIZE != (1 + sizeof(val))); + + insn[0] = 0xe9; /* Relative jump. */ + val = func->new_addr - func->old_addr - ARCH_PATCH_INSN_SIZE; + + memcpy(&insn[1], &val, sizeof(val)); + } + else + add_nops(insn, len); - *old_ptr++ = 0xe9; /* Relative jump */ - val = func->new_addr - func->old_addr - PATCH_INSN_SIZE; - memcpy(old_ptr, &val, sizeof(val)); + memcpy(old_ptr, insn, len); } void arch_livepatch_revert_jmp(const struct livepatch_func *func) { - memcpy(func->old_addr, func->opaque, PATCH_INSN_SIZE); + memcpy(func->old_addr, func->opaque, livepatch_insn_len(func)); } /* Serialise the CPU pipeline. */ diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index c9e5318..ed41f39 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -525,7 +525,8 @@ static int prepare_payload(struct payload *payload, return -EOPNOTSUPP; } - if ( !f->new_addr || !f->new_size ) + /* 'old_addr', 'new_addr', 'new_size' can all be zero. */ + if ( !f->old_size ) { dprintk(XENLOG_ERR, LIVEPATCH "%s: Address or size fields are zero!\n", elf->name); diff --git a/xen/include/asm-x86/alternative.h b/xen/include/asm-x86/alternative.h index 67fc0d2..db4f08e 100644 --- a/xen/include/asm-x86/alternative.h +++ b/xen/include/asm-x86/alternative.h @@ -27,6 +27,7 @@ struct alt_instr { #define ALT_ORIG_PTR(a) __ALT_PTR(a, instr_offset) #define ALT_REPL_PTR(a) __ALT_PTR(a, repl_offset) +extern void add_nops(void *insns, unsigned int len); /* Similar to alternative_instructions except it can be run with IRQs enabled. */ extern void apply_alternatives(const struct alt_instr *start, const struct alt_instr *end); diff --git a/xen/include/asm-x86/livepatch.h b/xen/include/asm-x86/livepatch.h new file mode 100644 index 0000000..5e04aa1 --- /dev/null +++ b/xen/include/asm-x86/livepatch.h @@ -0,0 +1,21 @@ +/* + * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved. + * + */ + +#ifndef __XEN_X86_LIVEPATCH_H__ +#define __XEN_X86_LIVEPATCH_H__ + +#define ARCH_PATCH_INSN_SIZE 5 + +#endif /* __XEN_X86_LIVEPATCH_H__ */ + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h index 29c9b31..174af06 100644 --- a/xen/include/xen/livepatch.h +++ b/xen/include/xen/livepatch.h @@ -68,7 +68,17 @@ 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 int arch_livepatch_verify_func(const struct livepatch_func *func); + +static inline +unsigned int livepatch_insn_len(const struct livepatch_func *func) +{ + if ( !func->new_addr ) + return func->new_size; + + return ARCH_PATCH_INSN_SIZE; +} /* * These functions are called around the critical region patching live code, * for an architecture to take make appropratie global state adjustments.