From patchwork Thu Aug 11 01:28:32 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: 9274393 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 D201A6022E for ; Thu, 11 Aug 2016 01:31:49 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B270128422 for ; Thu, 11 Aug 2016 01:31:49 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A5FDF2842A; Thu, 11 Aug 2016 01:31:49 +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 03ECA28422 for ; Thu, 11 Aug 2016 01:31:48 +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 1bXene-00061X-NR; Thu, 11 Aug 2016 01:28:46 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bXend-00061R-6N for xen-devel@lists.xenproject.org; Thu, 11 Aug 2016 01:28:45 +0000 Received: from [85.158.139.211] by server-13.bemta-5.messagelabs.com id B9/54-12874-CC4DBA75; Thu, 11 Aug 2016 01:28:44 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprHIsWRWlGSWpSXmKPExsUyZ7p8oO7pK6v DDTb1m1t83zKZyYHR4/CHKywBjFGsmXlJ+RUJrBlb2yaxFRwwqFg/9TlzA+NP5S5GLg4hgQ4m iR07NzJBON8YJZ42zmGEcDYySnQseMoC4fQwSux/tAyojJODRUBVYsGEo0AJDg42AROJN6scQ cIiArYSs6edZgGxhQV8JBr69jOD2LwCVhLth25A2YISJ2c+AathFtCSuPHvJRPIGGYBaYnl/z hAwqICyhINMx6AlUsIGEv0zepjmcDINwtJ9ywk3bMQuhcwMq9i1ChOLSpLLdI1MtFLKspMzyj JTczM0TU0MNXLTS0uTkxPzUlMKtZLzs/dxAgMt3oGBsYdjDcn+x1ilORgUhLlFY5ZHS7El5Sf UpmRWJwRX1Sak1p8iFGGg0NJgvfbZaCcYFFqempFWmYOMPBh0hIcPEoivL7A4BfiLS5IzC3OT IdInWJUlBLnfQjSJwCSyCjNg2uDRdslRlkpYV5GBgYGIZ6C1KLczBJU+VeM4hyMSsK8OiDjeT LzSuCmvwJazAS0OEl1BcjikkSElFQDI39Q0hyh1+u2BUz7su8kdz5/cpyz44VtQunb3L5d5PX g7Q4X5QyavGFmRUBe9/nJ35esEnC5m7/eSHimQJlcUsSfzyx9LLlGV2Z51FzYKXa34lNPaJF2 13luw1Wzf5S4u0aqps1In1i12m2avPfM8rD93msbOXKvrXl8yLDKt9P9w3GhfQlqSizFGYmGW sxFxYkAwrJvorECAAA= X-Env-Sender: konrad.wilk@oracle.com X-Msg-Ref: server-5.tower-206.messagelabs.com!1470878922!52303047!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 40894 invoked from network); 11 Aug 2016 01:28:43 -0000 Received: from userp1040.oracle.com (HELO userp1040.oracle.com) (156.151.31.81) by server-5.tower-206.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 11 Aug 2016 01:28:43 -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 u7B1Scj1028378 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 11 Aug 2016 01:28:38 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 u7B1SbJJ020530 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 11 Aug 2016 01:28:37 GMT Received: from abhmp0008.oracle.com (abhmp0008.oracle.com [141.146.116.14]) by aserv0121.oracle.com (8.13.8/8.13.8) with ESMTP id u7B1SaL7029239; Thu, 11 Aug 2016 01:28:37 GMT Received: from localhost.localdomain (/209.6.196.81) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 10 Aug 2016 18:28:36 -0700 Date: Wed, 10 Aug 2016 21:28:32 -0400 From: Konrad Rzeszutek Wilk To: ross.lagerwall@citrix.com, xen-devel@lists.xenproject.org Message-ID: <20160811012831.GB11611@localhost.localdomain> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.6.1 (2016-04-27) X-Source-IP: aserv0021.oracle.com [141.146.126.233] Subject: [Xen-devel] Livepatch, symbol resolutions between two livepatchs (new_symbol=0) 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: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP Hey Ross, I am running in a symbol dependency issue that I am not exactly sure how to solve. I have an payload that introduces a new function (xen_foobar) which will patch over xen_extra_version(). The loading says: (XEN) livepatch_elf.c:321: livepatch: xen_foobar: Symbol resolved: xen_foobar => 0xffff82d080409038 (.text) (XEN) livepatch.c:879: livepatch: xen_foobar: overriding symbol xen_foobar The later reason is b/c of: { bool_t found = 0; for ( j = 0; j < payload->nfuncs; j++ ) { if ( symtab[i].value == (unsigned long)payload->funcs[j].new_addr ) { found = 1; break; } } if ( !found ) { .... symtab[i].new_symbol = 1; } else { /* new_symbol is not set. */ ... Which makes sense - we do not want to introduce this symbol as a new one - as it has not been patched in. And there may be another payload which also has the same exact symbol to patch in. And either one may have the same exact .livepatch.depends (against the hypervisor) so either one could be loaded. Now imagine I have another payload which introduces xen_foobar2 which is suppose to patch over xen_foobar (bear with me, we could as well just do replace, but there is a good reason for this). Meaning the .livepatch.funcs has: .name = "xen_foobar", .new_addr = xen_foobar2, .old_addr = 0, (and the .livepatch.depends build-id is against one of the xen_foobar payloads). It gets loaded, and livepatch_elf_resolve_symbols is OK. But we choke in prepare_payload b/c of: f->old_addr = (void *)symbols_lookup_by_name(s); if ( !f->old_addr ) { f->old_addr = (void *)livepatch_symbols_lookup_by_name(s); if ( !f->old_addr ) { dprintk(XENLOG_ERR, LIVEPATCH "%s: Could not resolve old address of %s\n", elf->name, f->name); return -ENOENT; As livepatch_symbols_lookup_by_name only looks for symbols that have the ->new_symbol set. And xen_foobar does not. So the loading is aborted. Which makes sense - we don't want to match the symbols as they haven't really been "finally loaded" in. But what if the xen_foobar is applied. In that case we should change the xen_foobar to be new_symbol=1? This following patch does that, but I am wondering if there is a better way? P.S. The reason for this is that I am trying to implement NOP patching. And to have some regression testing of this I wrote an function (xen_foobar) which calls two functions: foo and bar - and their output is what the call to XENVER_extra_version will show (b/c we patch over xen_extra_version()). Then there is another payload - which will want to NOP the call to the 'bar' function inside xen_foobar. And for that I need to be able to lookup the symbol of xen_foobar. diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c index 1023fab..ad2011f 100644 --- a/xen/arch/x86/livepatch.c +++ b/xen/arch/x86/livepatch.c @@ -44,11 +44,11 @@ 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 > sizeof(func->u.s.opaque)); BUILD_BUG_ON(PATCH_INSN_SIZE != (1 + sizeof(val))); old_ptr = func->old_addr; - memcpy(func->opaque, old_ptr, PATCH_INSN_SIZE); + memcpy(func->u.s.opaque, old_ptr, PATCH_INSN_SIZE); *old_ptr++ = 0xe9; /* Relative jump */ val = func->new_addr - func->old_addr - PATCH_INSN_SIZE; @@ -57,7 +57,7 @@ void arch_livepatch_apply_jmp(struct livepatch_func *func) void arch_livepatch_revert_jmp(const struct livepatch_func *func) { - memcpy(func->old_addr, func->opaque, PATCH_INSN_SIZE); + memcpy(func->old_addr, func->u.s.opaque, PATCH_INSN_SIZE); } /* Serialise the CPU pipeline. */ diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index 5da28a3..4c11286 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -496,6 +496,8 @@ static int prepare_payload(struct payload *payload, if ( rc ) return rc; + f->u.s.idx = -1; + /* Lookup function's old address if not already resolved. */ if ( !f->old_addr ) { @@ -742,6 +744,7 @@ static int build_symbol_table(struct payload *payload, if ( symtab[i].value == (unsigned long)payload->funcs[j].new_addr ) { found = 1; + payload->funcs[j].u.s.idx = i; break; } } @@ -1000,8 +1003,21 @@ static int apply_payload(struct payload *data) arch_livepatch_quiesce(); for ( i = 0; i < data->nfuncs; i++ ) + { + int idx; + + idx = data->funcs[i].u.s.idx; + if ( idx >= 0 ) + { + struct livepatch_symbol *s; + + s = (struct livepatch_symbol *)&data->symtab[idx]; + s->new_symbol = 1; + } + arch_livepatch_apply_jmp(&data->funcs[i]); + } arch_livepatch_revive(); /* @@ -1023,7 +1039,19 @@ static int revert_payload(struct payload *data) arch_livepatch_quiesce(); for ( i = 0; i < data->nfuncs; i++ ) + { + int idx; + + idx = data->funcs[i].u.s.idx; + if ( idx >= 0 ) + { + struct livepatch_symbol *s; + + s = (struct livepatch_symbol *)&data->symtab[idx]; + s->new_symbol = 0; + } arch_livepatch_revert_jmp(&data->funcs[i]); + } arch_livepatch_revive(); diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h index 8197c14..162cd0f 100644 --- a/xen/include/public/sysctl.h +++ b/xen/include/public/sysctl.h @@ -884,7 +884,13 @@ struct livepatch_func { uint32_t new_size; uint32_t old_size; uint8_t version; /* MUST be LIVEPATCH_PAYLOAD_VERSION. */ - uint8_t opaque[31]; + union { + uint8_t opaque[31]; + struct { + int32_t idx; + uint8_t opaque[27]; + } s; + } u; }; typedef struct livepatch_func livepatch_func_t; #endif