From patchwork Wed Aug 21 08:19:18 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Wieczorkiewicz, Pawel" X-Patchwork-Id: 11105981 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9F4601399 for ; Wed, 21 Aug 2019 08:21:30 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 70CB52332A for ; Wed, 21 Aug 2019 08:21:30 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=amazon.de header.i=@amazon.de header.b="Tvr4PTJ6" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 70CB52332A Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=amazon.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1i0Lqq-00080d-2f; Wed, 21 Aug 2019 08:20:16 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1i0Lqp-00080J-AT for xen-devel@lists.xen.org; Wed, 21 Aug 2019 08:20:15 +0000 X-Inumbo-ID: 80a10312-c3ec-11e9-951b-bc764e2007e4 Received: from smtp-fw-4101.amazon.com (unknown [72.21.198.25]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 80a10312-c3ec-11e9-951b-bc764e2007e4; Wed, 21 Aug 2019 08:20:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.de; i=@amazon.de; q=dns/txt; s=amazon201209; t=1566375614; x=1597911614; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version; bh=CnbX1wDuqwsOIDTqfSPmgi15x7JFPFCyZA+MJm+f8/Q=; b=Tvr4PTJ6zO63TD/XGKW5kOVfCj/CnaMqVphcA8CMhNb2UG47+YLZQC6V ZE7S4Pel2HX+TtNz51NeBAE3PSdZDD84BzyfI8/fj/XRCrP5ogeW3ODC4 /TVYJ1aE6hvrw+bdyPvkfcLhEH/p8aGv/mNiLm/31s5HiF6CUIJL5fpv+ A=; X-IronPort-AV: E=Sophos;i="5.64,412,1559520000"; d="scan'208";a="780418881" Received: from iad6-co-svc-p1-lb1-vlan3.amazon.com (HELO email-inbound-relay-2b-a7fdc47a.us-west-2.amazon.com) ([10.124.125.6]) by smtp-border-fw-out-4101.iad4.amazon.com with ESMTP; 21 Aug 2019 08:20:12 +0000 Received: from EX13MTAUEA001.ant.amazon.com (pdx4-ws-svc-p6-lb7-vlan2.pdx.amazon.com [10.170.41.162]) by email-inbound-relay-2b-a7fdc47a.us-west-2.amazon.com (Postfix) with ESMTPS id BC36AC5DF6; Wed, 21 Aug 2019 08:20:11 +0000 (UTC) Received: from EX13D03EUC002.ant.amazon.com (10.43.164.60) by EX13MTAUEA001.ant.amazon.com (10.43.61.243) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Wed, 21 Aug 2019 08:19:58 +0000 Received: from EX13MTAUWB001.ant.amazon.com (10.43.161.207) by EX13D03EUC002.ant.amazon.com (10.43.164.60) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Wed, 21 Aug 2019 08:19:57 +0000 Received: from dev-dsk-wipawel-1a-0c4e6d58.eu-west-1.amazon.com (10.4.134.33) by mail-relay.amazon.com (10.43.161.249) with Microsoft SMTP Server id 15.0.1367.3 via Frontend Transport; Wed, 21 Aug 2019 08:19:53 +0000 From: Pawel Wieczorkiewicz To: , Date: Wed, 21 Aug 2019 08:19:18 +0000 Message-ID: <20190821081931.90887-2-wipawel@amazon.de> X-Mailer: git-send-email 2.16.5 In-Reply-To: <20190821081931.90887-1-wipawel@amazon.de> References: <20190821081931.90887-1-wipawel@amazon.de> MIME-Version: 1.0 Precedence: Bulk Subject: [Xen-devel] [PATCH 01/14] livepatch: Always check hypervisor build ID upon hotpatch upload X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: wipawel@amazon.com, Stefano Stabellini , Wei Liu , Konrad Rzeszutek Wilk , George Dunlap , Andrew Cooper , Ross Lagerwall , Ian Jackson , mpohlack@amazon.com, Tim Deegan , Pawel Wieczorkiewicz , Julien Grall , Jan Beulich Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" This change is part of a independant stacked hotpatch modules feature. This feature allows to bypass dependencies between modules upon loading, but still verifies Xen build ID matching. In order to prevent (up)loading any hotpatches built for different hypervisor version as indicated by the Xen Build ID, add checking for the payload's vs Xen's build id match. To achieve that embed into every hotpatch another section with a dedicated hypervisor build id in it. After the payload is loaded and the .livepatch.xen_depends section becomes available, perform the check and reject the payload if there is no match. Signed-off-by: Pawel Wieczorkiewicz Reviewed-by: Andra-Irina Paraschiv Reviewed-by: Bjoern Doebel Reviewed-by: Eslam Elnikety Reviewed-by: Martin Pohlack --- .gitignore | 1 + docs/misc/livepatch.pandoc | 28 +++++++++++++++++++-------- xen/common/livepatch.c | 47 +++++++++++++++++++++++++++++++++++++++++++++ xen/include/xen/livepatch.h | 7 ++++--- xen/test/livepatch/Makefile | 31 +++++++++++++++++++++++++----- 5 files changed, 98 insertions(+), 16 deletions(-) diff --git a/.gitignore b/.gitignore index 3c947ac948..6f83fc8728 100644 --- a/.gitignore +++ b/.gitignore @@ -312,6 +312,7 @@ xen/test/livepatch/xen_bye_world.livepatch xen/test/livepatch/xen_hello_world.livepatch xen/test/livepatch/xen_nop.livepatch xen/test/livepatch/xen_replace_world.livepatch +xen/test/livepatch/xen_no_xen_buildid.livepatch xen/tools/kconfig/.tmp_gtkcheck xen/tools/kconfig/.tmp_qtcheck xen/tools/symbols diff --git a/docs/misc/livepatch.pandoc b/docs/misc/livepatch.pandoc index 6d9f72f49b..fd1f5d0126 100644 --- a/docs/misc/livepatch.pandoc +++ b/docs/misc/livepatch.pandoc @@ -270,6 +270,8 @@ like what the Linux kernel module loader does. The payload contains at least three sections: * `.livepatch.funcs` - which is an array of livepatch_func structures. + * `.livepatch.xen_depends` - which is an ELF Note that describes what Xen + build-id the payload depends on. **MUST** have one. * `.livepatch.depends` - which is an ELF Note that describes what the payload depends on. **MUST** have one. * `.note.gnu.build-id` - the build-id of this payload. **MUST** have one. @@ -383,16 +385,16 @@ The type definition of the function are as follow: typedef void (*livepatch_loadcall_t)(void); typedef void (*livepatch_unloadcall_t)(void); -### .livepatch.depends and .note.gnu.build-id +### .livepatch.xen_depends, .livepatch.depends and .note.gnu.build-id To support dependencies checking and safe loading (to load the appropiate payload against the right hypervisor) there is a need to embbed an build-id dependency. -This is done by the payload containing an section `.livepatch.depends` -which follows the format of an ELF Note. The contents of this -(name, and description) are specific to the linker utilized to -build the hypevisor and payload. +This is done by the payload containing sections `.livepatch.xen_depends` +and `.livepatch.depends` which follow the format of an ELF Note. +The contents of these (name, and description) are specific to the linker +utilized to build the hypevisor and payload. If GNU linker is used then the name is `GNU` and the description is a NT_GNU_BUILD_ID type ID. The description can be an SHA1 @@ -400,6 +402,13 @@ checksum, MD5 checksum or any unique value. The size of these structures varies with the `--build-id` linker option. +There are two kinds of build-id dependencies: + + * Xen build-id dependency (.livepatch.xen_depends section) + * previous payload build-id dependency (.livepatch.depends section) + +See "Live patch interdependencies" for more information. + ## Hypercalls We will employ the sub operations of the system management hypercall (sysctl). @@ -894,13 +903,16 @@ but is more complex to implement. The second option which requires an build-id of the hypervisor is implemented in the Xen hypervisor. -Specifically each payload has two build-id ELF notes: +Specifically each payload has three build-id ELF notes: * The build-id of the payload itself (generated via --build-id). + * The build-id of the Xen hypervisor it depends on (extracted from the + hypervisor during build time). * The build-id of the payload it depends on (extracted from the the previous payload or hypervisor during build time). -This means that the very first payload depends on the hypervisor -build-id. +This means that every payload depends on the hypervisor build-id and on +the build-id of the previous payload in the stack. +The very first payload depends on the hypervisor build-id only. # Not Yet Done diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index d6eaae6d3b..6a4af6ce57 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -74,6 +74,7 @@ struct payload { unsigned int nsyms; /* Nr of entries in .strtab and symbols. */ struct livepatch_build_id id; /* ELFNOTE_DESC(.note.gnu.build-id) of the payload. */ struct livepatch_build_id dep; /* ELFNOTE_DESC(.livepatch.depends). */ + struct livepatch_build_id xen_dep; /* ELFNOTE_DESC(.livepatch.xen_depends). */ livepatch_loadcall_t *const *load_funcs; /* The array of funcs to call after */ livepatch_unloadcall_t *const *unload_funcs;/* load and unload of the payload. */ unsigned int n_load_funcs; /* Nr of the funcs to load and execute. */ @@ -476,11 +477,34 @@ static bool section_ok(const struct livepatch_elf *elf, return true; } +static int check_xen_build_id(const struct payload *payload) +{ + const void *id = NULL; + unsigned int len = 0; + int rc; + + ASSERT(payload->xen_dep.len); + ASSERT(payload->xen_dep.p); + + rc = xen_build_id(&id, &len); + if ( rc ) + return rc; + + if ( payload->xen_dep.len != len || memcmp(id, payload->xen_dep.p, len) ) { + dprintk(XENLOG_ERR, "%s%s: check against hypervisor build-id failed!\n", + LIVEPATCH, payload->name); + return -EINVAL; + } + + return 0; +} + static int check_special_sections(const struct livepatch_elf *elf) { unsigned int i; static const char *const names[] = { ELF_LIVEPATCH_FUNC, ELF_LIVEPATCH_DEPENDS, + ELF_LIVEPATCH_XEN_DEPENDS, ELF_BUILD_ID_NOTE}; DECLARE_BITMAP(found, ARRAY_SIZE(names)) = { 0 }; @@ -632,6 +656,22 @@ static int prepare_payload(struct payload *payload, return -EINVAL; } + sec = livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_XEN_DEPENDS); + if ( sec ) + { + n = sec->load_addr; + + if ( sec->sec->sh_size <= sizeof(*n) ) + return -EINVAL; + + if ( xen_build_id_check(n, sec->sec->sh_size, + &payload->xen_dep.p, &payload->xen_dep.len) ) + return -EINVAL; + + if ( !payload->xen_dep.len || !payload->xen_dep.p ) + return -EINVAL; + } + /* Setup the virtual region with proper data. */ region = &payload->region; @@ -882,6 +922,10 @@ static int load_payload_data(struct payload *payload, void *raw, size_t len) if ( rc ) goto out; + rc = check_xen_build_id(payload); + if ( rc ) + goto out; + rc = build_symbol_table(payload, &elf); if ( rc ) goto out; @@ -1655,6 +1699,9 @@ static void livepatch_printall(unsigned char key) if ( data->dep.len ) printk("depend-on=%*phN\n", data->dep.len, data->dep.p); + + if ( data->xen_dep.len ) + printk("depend-on-xen=%*phN\n", data->xen_dep.len, data->xen_dep.p); } spin_unlock(&payload_lock); diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h index 1b1817ca0d..ed997aa4cc 100644 --- a/xen/include/xen/livepatch.h +++ b/xen/include/xen/livepatch.h @@ -29,9 +29,10 @@ struct xen_sysctl_livepatch_op; /* Convenience define for printk. */ #define LIVEPATCH "livepatch: " /* ELF payload special section names. */ -#define ELF_LIVEPATCH_FUNC ".livepatch.funcs" -#define ELF_LIVEPATCH_DEPENDS ".livepatch.depends" -#define ELF_BUILD_ID_NOTE ".note.gnu.build-id" +#define ELF_LIVEPATCH_FUNC ".livepatch.funcs" +#define ELF_LIVEPATCH_DEPENDS ".livepatch.depends" +#define ELF_LIVEPATCH_XEN_DEPENDS ".livepatch.xen_depends" +#define ELF_BUILD_ID_NOTE ".note.gnu.build-id" /* Arbitrary limit for payload size and .bss section size. */ #define LIVEPATCH_MAX_SIZE MB(2) diff --git a/xen/test/livepatch/Makefile b/xen/test/livepatch/Makefile index 6831383db1..fdb82782d2 100644 --- a/xen/test/livepatch/Makefile +++ b/xen/test/livepatch/Makefile @@ -19,11 +19,13 @@ LIVEPATCH := xen_hello_world.livepatch LIVEPATCH_BYE := xen_bye_world.livepatch LIVEPATCH_REPLACE := xen_replace_world.livepatch LIVEPATCH_NOP := xen_nop.livepatch +LIVEPATCH_NO_XEN_BUILDID := xen_no_xen_buildid.livepatch LIVEPATCHES += $(LIVEPATCH) LIVEPATCHES += $(LIVEPATCH_BYE) LIVEPATCHES += $(LIVEPATCH_REPLACE) LIVEPATCHES += $(LIVEPATCH_NOP) +LIVEPATCHES += $(LIVEPATCH_NO_XEN_BUILDID) LIVEPATCH_DEBUG_DIR ?= $(DEBUG_DIR)/xen-livepatch @@ -59,7 +61,7 @@ config.h: xen_hello_world_func.o xen_hello_world.o: config.h .PHONY: $(LIVEPATCH) -$(LIVEPATCH): xen_hello_world_func.o xen_hello_world.o note.o +$(LIVEPATCH): xen_hello_world_func.o xen_hello_world.o note.o xen_note.o $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH) $^ # @@ -78,6 +80,17 @@ note.o: --rename-section=.data=.livepatch.depends,alloc,load,readonly,data,contents -S $@.bin $@ rm -f $@.bin +# +# Append .livepatch.xen_depends section +# with Xen build-id derived from xen-syms. +# +.PHONY: xen_note.o +xen_note.o: + $(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(BASEDIR)/xen-syms $@.bin + $(OBJCOPY) $(OBJCOPY_MAGIC) \ + --rename-section=.data=.livepatch.xen_depends,alloc,load,readonly,data,contents -S $@.bin $@ + rm -f $@.bin + # # Extract the build-id of the xen_hello_world.livepatch # (which xen_bye_world will depend on). @@ -92,20 +105,28 @@ hello_world_note.o: $(LIVEPATCH) xen_bye_world.o: config.h .PHONY: $(LIVEPATCH_BYE) -$(LIVEPATCH_BYE): xen_bye_world_func.o xen_bye_world.o hello_world_note.o +$(LIVEPATCH_BYE): xen_bye_world_func.o xen_bye_world.o hello_world_note.o xen_note.o $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_BYE) $^ xen_replace_world.o: config.h .PHONY: $(LIVEPATCH_REPLACE) -$(LIVEPATCH_REPLACE): xen_replace_world_func.o xen_replace_world.o note.o +$(LIVEPATCH_REPLACE): xen_replace_world_func.o xen_replace_world.o note.o xen_note.o $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_REPLACE) $^ xen_nop.o: config.h .PHONY: $(LIVEPATCH_NOP) -$(LIVEPATCH_NOP): xen_nop.o note.o +$(LIVEPATCH_NOP): xen_nop.o note.o xen_note.o $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_NOP) $^ +# This one always fails upon upload, because it deliberetely +# does not have a .livepatch.xen_depends (xen_note.o) section. +xen_no_xen_buildid.o: config.h + +.PHONY: $(LIVEPATCH_NO_XEN_BUILDID) +$(LIVEPATCH_NO_XEN_BUILDID): xen_nop.o note.o + $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_NO_XEN_BUILDID) $^ + .PHONY: livepatch -livepatch: $(LIVEPATCH) $(LIVEPATCH_BYE) $(LIVEPATCH_REPLACE) $(LIVEPATCH_NOP) +livepatch: $(LIVEPATCH) $(LIVEPATCH_BYE) $(LIVEPATCH_REPLACE) $(LIVEPATCH_NOP) $(LIVEPATCH_NO_XEN_BUILDID)