From patchwork Fri Sep 23 16:15:04 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: 9348447 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 05BF0607F2 for ; Fri, 23 Sep 2016 16:18:06 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E9FB92AC15 for ; Fri, 23 Sep 2016 16:18:05 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DE5DA2AC5A; Fri, 23 Sep 2016 16:18:05 +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 DA89A2AC15 for ; Fri, 23 Sep 2016 16:18:04 +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 1bnT8C-0002w1-HD; Fri, 23 Sep 2016 16:15:20 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bnT8B-0002ve-A8 for xen-devel@lists.xenproject.org; Fri, 23 Sep 2016 16:15:19 +0000 Received: from [85.158.139.211] by server-5.bemta-5.messagelabs.com id 35/8A-30284-61555E75; Fri, 23 Sep 2016 16:15:18 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrHIsWRWlGSWpSXmKPExsUyZ7p8oK5Y6NN wg6+7uS2+b5nM5MDocfjDFZYAxijWzLyk/IoE1owpdxczF/QaVbxf/5y1gXGRehcjF4eQwGQm ibZZu1ghnG+MEn/3TmSBcDYwSix9e4sNwulmlJi8fhWQwwHkFEmc+6jfxcjJwSKgKjHr6UVmk DCbgInEm1WOIGERAT2JrjXNzCCtzAL7GSVmPpnNCJIQFoiWuPd3HZjNK2AusfXfb0aI+VsZJR Yd2MEEkRCUODnzCQuIzSygJXHj30smkAXMAtISy/9xgIQ5Bewk1nYsZgaxRQWUJRb397CB2BI CxhLtby+yTWAUmoVk0iwkk2YhTFrAyLyKUb04tagstUjXXC+pKDM9oyQ3MTNH19DAVC83tbg4 MT01JzGpWC85P3cTIzCcGYBgB+Oxyc6HGCU5mJREeZXcnoYL8SXlp1RmJBZnxBeV5qQWH2KU4 eBQkuB1MADKCRalpqdWpGXmACMLJi3BwaMkwnswGCjNW1yQmFucmQ6ROsWoKCXOuxwkIQCSyC jNg2uDRfMlRlkpYV5GoEOEeApSi3IzS1DlXzGKczAqCfNqhQBN4cnMK4Gb/gpoMRPQ4m93noA sLklESEk1MK6sCw8wvtJ53L/rZ7rrcePED02a+p3f7554NWGVf6qjRRCHC//xTLld/ydE5Lv8 cGIzz2/leH3497dZEg4CTR8iL/U4l9p+2pikcNV6XuiUPxsOf7D/E5DAaxHsdJnryRoj7lpBc 6lWXieNC6dub1m9v8IhzPxdAHuutucj0eq42N7N2Yv7lViKMxINtZiLihMBfuXxh+ECAAA= X-Env-Sender: konrad.wilk@oracle.com X-Msg-Ref: server-7.tower-206.messagelabs.com!1474647316!60981510!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 3543 invoked from network); 23 Sep 2016 16:15:17 -0000 Received: from userp1040.oracle.com (HELO userp1040.oracle.com) (156.151.31.81) by server-7.tower-206.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 23 Sep 2016 16:15:17 -0000 Received: from aserv0022.oracle.com (aserv0022.oracle.com [141.146.126.234]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id u8NGF8Cn001408 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 23 Sep 2016 16:15:08 GMT Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by aserv0022.oracle.com (8.14.4/8.14.4) with ESMTP id u8NGF8Oa008335 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 23 Sep 2016 16:15:08 GMT Received: from abhmp0003.oracle.com (abhmp0003.oracle.com [141.146.116.9]) by aserv0121.oracle.com (8.13.8/8.13.8) with ESMTP id u8NGF5R8019774; Fri, 23 Sep 2016 16:15:06 GMT Received: from char.us.oracle.com (/10.137.176.158) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Fri, 23 Sep 2016 09:15:05 -0700 Received: by char.us.oracle.com (Postfix, from userid 1000) id 856BB6A0DEF; Fri, 23 Sep 2016 12:15:04 -0400 (EDT) Date: Fri, 23 Sep 2016 12:15:04 -0400 From: Konrad Rzeszutek Wilk To: Ross Lagerwall Message-ID: <20160923161504.GG25028@char.us.oracle.com> References: <1474479154-20991-1-git-send-email-konrad.wilk@oracle.com> <1474479154-20991-9-git-send-email-konrad.wilk@oracle.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.2 (2016-07-01) X-Source-IP: aserv0022.oracle.com [141.146.126.234] Cc: sstabellini@kernel.org, Andrew Cooper , julien.grall@arm.com, Jan Beulich , xen-devel@lists.xenproject.org Subject: Re: [Xen-devel] [PATCH v5 08/16] livepatch/arm/x86: Check payload for for unwelcomed symbols. 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 > > 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_elf.c b/xen/common/livepatch_elf.c > > index f46990e..ec74beb 100644 > > --- a/xen/common/livepatch_elf.c > > +++ b/xen/common/livepatch_elf.c > > @@ -251,6 +251,13 @@ static int elf_get_sym(struct livepatch_elf *elf, const void *data) > > > > sym[i].sym = s; > > sym[i].name = strtab_sec->data + delta; > > + /* e.g. On ARM we should NEVER see $t* symbols. */ > > I'd prefer this comment in ARM's arch_livepatch_symbol_deny. I removed the comment from common code. With the latest patch each architecture has its own implementation of arch_livepatch_symbol_deny. > > Either way, > Reviewed-by: Ross Lagerwall Thanks. Here is how the updated patch looks like: From b84b478c4214f97e809f659fe348493c48a51d0c Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Tue, 13 Sep 2016 13:15:07 -0400 Subject: [PATCH v6] livepatch/arm/x86: Check payload for for unwelcomed symbols. Certain platforms, such as ARM [32|64] add extra mapping symbols such as $x (for ARM64 instructions), or more interesting to this patch: $t (for Thumb instructions). These symbols are supposed to help the final linker to make any adjustments (such as add an veneer). But more importantly - we do not compile Xen with any Thumb instructions (which are variable length) - and if we find these mapping symbols we should disallow such payload. Reviewed-by: Ross Lagerwall Reviewed-by: Julien Grall Signed-off-by: Konrad Rzeszutek Wilk --- Cc: Konrad Rzeszutek Wilk Cc: Ross Lagerwall Cc: Stefano Stabellini Cc: Julien Grall Cc: Andrew Cooper v3: New submission. Use &sym[i] instead of sym (as that will always be NULL). v4: Use bool instead of int for return Update comment in common code about ARM odd symbols. s/_check/_deny/ to make it more clear. v5: Also check for $t.* wildcard. Use Julien's variant where we roll the [2] check in the return. v6: s/suppose/supposed/ in commit description. Move arch_livepatch_symbol_deny in arm[32|64]/livepatch.c Added Julien's Reviewed-by. Added Ross's Reviewed-by. Removed comment from common code talking about $t symbol. --- xen/arch/arm/arm32/livepatch.c | 13 +++++++++++++ xen/arch/arm/arm64/livepatch.c | 7 +++++++ xen/arch/x86/livepatch.c | 7 +++++++ xen/common/livepatch_elf.c | 6 ++++++ xen/include/xen/livepatch.h | 2 ++ 5 files changed, 35 insertions(+) diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c index 80f9646..5fc2e63 100644 --- a/xen/arch/arm/arm32/livepatch.c +++ b/xen/arch/arm/arm32/livepatch.c @@ -20,6 +20,19 @@ int arch_livepatch_verify_elf(const struct livepatch_elf *elf) return -EOPNOTSUPP; } +bool arch_livepatch_symbol_deny(const struct livepatch_elf *elf, + const struct livepatch_elf_sym *sym) +{ + /* + * Xen does not use Thumb instructions - and we should not see any of + * them. If we do, abort. + */ + if ( sym->name && sym->name[0] == '$' && sym->name[1] == 't' ) + return ( !sym->name[2] || sym->name[2] == '.' ); + + return false; +} + int arch_livepatch_perform_rela(struct livepatch_elf *elf, const struct livepatch_elf_sec *base, const struct livepatch_elf_sec *rela) diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c index 774f845..f148927 100644 --- a/xen/arch/arm/arm64/livepatch.c +++ b/xen/arch/arm/arm64/livepatch.c @@ -90,6 +90,13 @@ int arch_livepatch_verify_elf(const struct livepatch_elf *elf) return 0; } +bool arch_livepatch_symbol_deny(const struct livepatch_elf *elf, + const struct livepatch_elf_sym *sym) +{ + /* No special checks on ARM 64. */ + 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 7a369a0..9663ef6 100644 --- a/xen/arch/x86/livepatch.c +++ b/xen/arch/x86/livepatch.c @@ -131,6 +131,13 @@ bool arch_livepatch_symbol_ok(const struct livepatch_elf *elf, return true; } +bool arch_livepatch_symbol_deny(const struct livepatch_elf *elf, + const struct livepatch_elf_sym *sym) +{ + /* No special checks on x86. */ + 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_elf.c b/xen/common/livepatch_elf.c index dec904a..c4a9633 100644 --- a/xen/common/livepatch_elf.c +++ b/xen/common/livepatch_elf.c @@ -251,6 +251,12 @@ static int elf_get_sym(struct livepatch_elf *elf, const void *data) sym[i].sym = s; sym[i].name = strtab_sec->data + delta; + if ( arch_livepatch_symbol_deny(elf, &sym[i]) ) + { + dprintk(XENLOG_ERR, LIVEPATCH "%s: Symbol '%s' should not be in payload!\n", + elf->name, sym[i].name); + return -EINVAL; + } } elf->nsym = nsym; diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h index e8c67d6..98ec012 100644 --- a/xen/include/xen/livepatch.h +++ b/xen/include/xen/livepatch.h @@ -50,6 +50,8 @@ bool_t is_patch(const void *addr); int arch_livepatch_verify_elf(const struct livepatch_elf *elf); bool arch_livepatch_symbol_ok(const struct livepatch_elf *elf, const struct livepatch_elf_sym *sym); +bool arch_livepatch_symbol_deny(const struct livepatch_elf *elf, + const struct livepatch_elf_sym *sym); int arch_livepatch_perform_rel(struct livepatch_elf *elf, const struct livepatch_elf_sec *base, const struct livepatch_elf_sec *rela);