From patchwork Sat Sep 28 15:12:54 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Wieczorkiewicz, Pawel" X-Patchwork-Id: 11165441 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 903DC912 for ; Sat, 28 Sep 2019 15:18:22 +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 5FFC720880 for ; Sat, 28 Sep 2019 15:18:22 +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="lwhJMArQ" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5FFC720880 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 1iEET7-0001Xv-Jp; Sat, 28 Sep 2019 15:17:09 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iEET5-0001Xf-W7 for xen-devel@lists.xenproject.org; Sat, 28 Sep 2019 15:17:08 +0000 X-Inumbo-ID: 088399e6-e203-11e9-bf31-bc764e2007e4 Received: from smtp-fw-9102.amazon.com (unknown [207.171.184.29]) by localhost (Halon) with ESMTPS id 088399e6-e203-11e9-bf31-bc764e2007e4; Sat, 28 Sep 2019 15:17:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.de; i=@amazon.de; q=dns/txt; s=amazon201209; t=1569683827; x=1601219827; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version; bh=SCOh7uYNpBvxg78vVImipiy3W/vveYwwu2WpG6XFJzg=; b=lwhJMArQaAB1L+jv/heuyszTA+fZB9oV9DfpMrJWoakQ33FeTFc2uSH1 85xdtagRuHNeo3UFDaFDTO3jtKPu4KpkoVn65dPbeEHGHIb/WXUEsKMzB BrxCW5o6kkvIAkAOnQlZLyVL8OC1ndKe59BfS0pFXE6fc283HmEEcTSH6 8=; X-IronPort-AV: E=Sophos;i="5.64,559,1559520000"; d="scan'208";a="705176494" Received: from sea3-co-svc-lb6-vlan3.sea.amazon.com (HELO email-inbound-relay-1a-67b371d8.us-east-1.amazon.com) ([10.47.22.38]) by smtp-border-fw-out-9102.sea19.amazon.com with ESMTP; 28 Sep 2019 15:13:57 +0000 Received: from EX13MTAUEA001.ant.amazon.com (iad55-ws-svc-p15-lb9-vlan3.iad.amazon.com [10.40.159.166]) by email-inbound-relay-1a-67b371d8.us-east-1.amazon.com (Postfix) with ESMTPS id 7483AA221B; Sat, 28 Sep 2019 15:13:37 +0000 (UTC) Received: from EX13D03EUA002.ant.amazon.com (10.43.165.166) by EX13MTAUEA001.ant.amazon.com (10.43.61.82) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Sat, 28 Sep 2019 15:13:28 +0000 Received: from EX13MTAUWB001.ant.amazon.com (10.43.161.207) by EX13D03EUA002.ant.amazon.com (10.43.165.166) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Sat, 28 Sep 2019 15:13:27 +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; Sat, 28 Sep 2019 15:13:23 +0000 From: Pawel Wieczorkiewicz To: Date: Sat, 28 Sep 2019 15:12:54 +0000 Message-ID: <20190928151305.127380-2-wipawel@amazon.de> X-Mailer: git-send-email 2.16.5 In-Reply-To: <20190928151305.127380-1-wipawel@amazon.de> References: <20190928151305.127380-1-wipawel@amazon.de> MIME-Version: 1.0 Precedence: Bulk Subject: [Xen-devel] [PATCH v4 01/12] livepatch: Always check hypervisor build ID upon livepatch 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 livepatch 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 livepatches 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 livepatch 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 Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: Ross Lagerwall --- Changed since v3: * renamed check_xen_build_id() to xen_build_id_dep() Changed since v1: * always print XENLOG_ERR message from check_xen_build_id() * fix typo in test/livepatch/Makefile .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 7caa30c202..163c9c79ea 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 xen_build_id_dep(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) ) { + printk(XENLOG_ERR LIVEPATCH "%s: check against hypervisor build-id failed\n", + 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 = xen_build_id_dep(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..938aee17ec 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 deliberately +# 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)