From patchwork Fri Sep 16 15:29:09 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: 9336127 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 2EB20608A0 for ; Fri, 16 Sep 2016 15:31:55 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1DB5929F43 for ; Fri, 16 Sep 2016 15:31:55 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 127222A014; Fri, 16 Sep 2016 15:31:55 +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 897B12A015 for ; Fri, 16 Sep 2016 15:31:54 +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 1bkv4y-0003xf-C4; Fri, 16 Sep 2016 15:29:28 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bkv4w-0003xT-Fn for xen-devel@lists.xenproject.org; Fri, 16 Sep 2016 15:29:26 +0000 Received: from [85.158.139.211] by server-12.bemta-5.messagelabs.com id 94/8B-09561-5DF0CD75; Fri, 16 Sep 2016 15:29:25 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprEIsWRWlGSWpSXmKPExsXSO6nOVfcq/51 wgzcbuC2+b5nM5MDocfjDFZYAxijWzLyk/IoE1oxJm/YwF6xQqfj0S6mB8YRUFyMXh5BAG5PE 0h2L2CCcb4wS/x5uY4ZwNjBKrLn9GirTzSjxZPUL9i5GTiCnSOJ5xzWWLkYODjYBE4k3qxxBw iICxRKbL91lArGZBaIlJvW1sYHYwgIuEhsnz2YEsVkEVCU+z7nCCNLKK+AmMfmJNUhYQkBOYt uWPWAlnALuEqe+rmeE2OQmseTBZzaIGkOJ0w+3MU5g5F/AyLCKUaM4tagstUjXyEIvqSgzPaM kNzEzR9fQwFQvN7W4ODE9NScxqVgvOT93EyMwfOoZGBh3MPat8jvEKMnBpCTKW1x4O1yILyk/ pTIjsTgjvqg0J7X4EKMMB4eSBO8HvjvhQoJFqempFWmZOcBAhklLcPAoifAqAYNZiLe4IDG3O DMdInWKUZejq/P5OSYhlrz8vFQpcd6/IDMEQIoySvPgRsCi6hKjrJQwLyMDA4MQT0FqUW5mCa r8K0ZxDkYlYV5zkFU8mXklcJteAR3BBHTE6lm3QY4oSURISTUwsnMmvz8n7LsmZKbJlrc3u3v Mjdpk1t3fsHXl1Uo1DT4G1Zjashcxj2e/0pvguHDn7mwLubqbM4qC5BWeFfzYHKYgtIl9OsuS fRwHDkoq7O3a/CfXyXT9qn/b50zT/Sd4JDtz1s13/xylEzgqTEXvK0jsPT+lx3mfyuIFccGKz gvs5ld2WRUvVGIpzkg01GIuKk4EAODyWPylAgAA X-Env-Sender: konrad@char.us.oracle.com X-Msg-Ref: server-8.tower-206.messagelabs.com!1474039763!59814493!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 35260 invoked from network); 16 Sep 2016 15:29:24 -0000 Received: from aserp1040.oracle.com (HELO aserp1040.oracle.com) (141.146.126.69) by server-8.tower-206.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 16 Sep 2016 15:29:24 -0000 Received: from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71]) by aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id u8GFTIGE004807 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 16 Sep 2016 15:29:19 GMT Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by userv0021.oracle.com (8.13.8/8.13.8) with ESMTP id u8GFTI05002906 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 16 Sep 2016 15:29:18 GMT Received: from abhmp0001.oracle.com (abhmp0001.oracle.com [141.146.116.7]) by aserv0121.oracle.com (8.13.8/8.13.8) with ESMTP id u8GFTHBx029044; Fri, 16 Sep 2016 15:29:17 GMT Received: from char.us.oracle.com (/10.137.176.158) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Fri, 16 Sep 2016 08:29:17 -0700 Received: by char.us.oracle.com (Postfix, from userid 1000) id 648596A0DDD; Fri, 16 Sep 2016 11:29:15 -0400 (EDT) From: Konrad Rzeszutek Wilk To: xen-devel@lists.xenproject.org, konrad@kernel.org, ross.lagerwall@citrix.com, andrew.cooper3@citrix.com Date: Fri, 16 Sep 2016 11:29:09 -0400 Message-Id: <1474039754-25816-2-git-send-email-konrad.wilk@oracle.com> X-Mailer: git-send-email 2.5.5 In-Reply-To: <1474039754-25816-1-git-send-email-konrad.wilk@oracle.com> References: <1474039754-25816-1-git-send-email-konrad.wilk@oracle.com> X-Source-IP: userv0021.oracle.com [156.151.31.71] Cc: Jan Beulich , Konrad Rzeszutek Wilk Subject: [Xen-devel] [PATCH v6 1/6] livepatch: Disallow applying after an revert 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 On general this is unhealthy - as the payload's .bss (definitly) or .data (maybe) will be modified once the payload is running. Doing an revert and then re-applying the payload with a non-pristine .bss or .data can lead to unforseen consequences (.bss are assumed to always contain zero value but now they may have a different value). There is one exception - if the payload contains only one .data section - the .livepatch.funcs, then it is OK to re-apply an revert. Suggested-by: Jan Beulich Signed-off-by: Konrad Rzeszutek Wilk --- Cc: Andrew Cooper Cc: Jan Beulich v6: New submission --- docs/misc/livepatch.markdown | 7 +++++++ xen/common/livepatch.c | 27 ++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/docs/misc/livepatch.markdown b/docs/misc/livepatch.markdown index 89c1050..81f4fc9 100644 --- a/docs/misc/livepatch.markdown +++ b/docs/misc/livepatch.markdown @@ -1061,6 +1061,13 @@ depending on the current state of data. As such it should not be attempted. That said we should provide hook functions so that the existing data can be changed during payload application. +To guarantee safety we disallow re-applying an payload after it has been +reverted. This is because we cannot guarantee that the state of .bss +and .data to be exactly as it was during loading. Hence the administrator +MUST unload the payload and upload it again to apply it. + +There is an exception to this: if the payload only has .livepatch.funcs; +and no .data or .bss sections. ### Inline patching diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index 23e4d51..967985c 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -52,6 +52,8 @@ struct livepatch_build_id { struct payload { uint32_t state; /* One of the LIVEPATCH_STATE_*. */ int32_t rc; /* 0 or -XEN_EXX. */ + bool_t reverted; /* Whether it was reverted. */ + bool_t safe_to_reapply; /* Can apply safely after revert. */ struct list_head list; /* Linked to 'payload_list'. */ const void *text_addr; /* Virtual address of .text. */ size_t text_size; /* .. and its size. */ @@ -308,7 +310,7 @@ static void calc_section(const struct livepatch_elf_sec *sec, size_t *size, static int move_payload(struct payload *payload, struct livepatch_elf *elf) { void *text_buf, *ro_buf, *rw_buf; - unsigned int i; + unsigned int i, rw_buf_sec, rw_buf_cnt = 0; size_t size = 0; unsigned int *offset; int rc = 0; @@ -381,7 +383,11 @@ static int move_payload(struct payload *payload, struct livepatch_elf *elf) if ( elf->sec[i].sec->sh_flags & SHF_EXECINSTR ) buf = text_buf; else if ( elf->sec[i].sec->sh_flags & SHF_WRITE ) + { buf = rw_buf; + rw_buf_sec = i; + rw_buf_cnt++; + } else buf = ro_buf; @@ -402,6 +408,10 @@ static int move_payload(struct payload *payload, struct livepatch_elf *elf) } } + /* No .bss and no .data, and only on RW - .livepatch.funcs section. */ + if ( rw_buf_cnt == 1 && + !strcmp(elf->sec[rw_buf_sec].name, ELF_LIVEPATCH_FUNC) ) + payload->safe_to_reapply = true; out: xfree(offset); @@ -1057,6 +1067,7 @@ static int revert_payload(struct payload *data) list_del_rcu(&data->applied_list); unregister_virtual_region(&data->region); + data->reverted = true; return 0; } @@ -1438,6 +1449,20 @@ static int livepatch_action(xen_sysctl_livepatch_action_t *action) case LIVEPATCH_ACTION_APPLY: if ( data->state == LIVEPATCH_STATE_CHECKED ) { + /* + * It is unsafe to apply an reverted payload as the .data (or .bss) + * may not be in in pristine condition. Hence MUST unload and then + * apply patch again. Unless the payload has no .bss or only one + * RW section (.livepatch.funcs). + */ + if ( data->reverted && !data->safe_to_reapply ) + { + dprintk(XENLOG_ERR, "%s%s: can't revert as payload has .data. Please unload!\n", + LIVEPATCH, data->name); + data->rc = -EINVAL; + break; + } + rc = build_id_dep(data, !!list_empty(&applied_list)); if ( rc ) break;