From patchwork Wed Apr 27 16:36:19 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: 8960211 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 488739F1C1 for ; Wed, 27 Apr 2016 16:39:00 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 3302020219 for ; Wed, 27 Apr 2016 16:38:58 +0000 (UTC) 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.kernel.org (Postfix) with ESMTPS id A93CF20121 for ; Wed, 27 Apr 2016 16:38:55 +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 1avSRw-0006B1-Ei; Wed, 27 Apr 2016 16:36:28 +0000 Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1avSRu-0006Av-Pp for xen-devel@lists.xenproject.org; Wed, 27 Apr 2016 16:36:27 +0000 Received: from [193.109.254.147] by server-6.bemta-14.messagelabs.com id 18/10-03753-A8AE0275; Wed, 27 Apr 2016 16:36:26 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrFIsWRWlGSWpSXmKPExsVyMfTOId3OVwr hBhNOMll83zKZyYHR4/CHKywBjFGsmXlJ+RUJrBlTulewFxy7zVjxe/YmpgbGr2sYuxi5OIQE ZjBKHL0zgx3EYRFYxioxce46sIyEwCFWibZFx1i6GDmBnBiJ1ccnMXUxcgDZ1RI/ZpmBhIUEl CS2TH4MNWkCk8Ssf0uYQBLCAnoSk7/dZgSxWQRUJZb8uQg2h01AX+Lp2mvMILaIgLJE76/fLC DNzAKXGSXmd/5kg2j2lXjRfQpsEK+AmUTro0+sEBu2Mkp0nb3BCpEQlDg58wnYVGYBLYkb/16 CXccsIC2x/B8HiMkpYC/RNcEAxBQVUJF4dbB+AqPILCS9s5D0zkLoXcDIvIpRozi1qCy1SNfQ Qi+pKDM9oyQ3MTNH19DQRC83tbg4MT01JzGpWC85P3cTIzAG6hkYGHcwHtnueYhRkoNJSZR3y VmFcCG+pPyUyozE4oz4otKc1OJDjDIcHEoSvDNfAuUEi1LTUyvSMnOA0QiTluDgURLhPQKS5i 0uSMwtzkyHSJ1iNObY8vvaWiaObVPvrWUSYsnLz0uVEuedDVIqAFKaUZoHNwiWJC4xykoJ8zI yMDAI8RSkFuVmlqDKv2IU52BUEuadBTKFJzOvBG7fK6BTmIBOuXxIFuSUkkSElFQDY8zHT09P RM+/yK+hG9XY9vL9s+61c3q5GxapbStbNX930kq+uREt/HGzGN5/YVXqXc2zQDbueBp7V4NA0 0T9dTeszjuqH9y7TuKLaPKRHRMyI/cJXDoil6QmMPnubOdLL9ZcvPajJPLUlt9Peb4WCO3q/G b42IBD7cHytrqm9BbxnNjfVzyMHZVYijMSDbWYi4oTAZGnogMNAwAA X-Env-Sender: ketuzsezr@gmail.com X-Msg-Ref: server-14.tower-27.messagelabs.com!1461774983!25982017!1 X-Originating-IP: [209.85.220.194] X-SpamReason: No, hits=0.5 required=7.0 tests=BODY_RANDOM_LONG X-StarScan-Received: X-StarScan-Version: 8.34; banners=-,-,- X-VirusChecked: Checked Received: (qmail 19948 invoked from network); 27 Apr 2016 16:36:24 -0000 Received: from mail-qk0-f194.google.com (HELO mail-qk0-f194.google.com) (209.85.220.194) by server-14.tower-27.messagelabs.com with AES128-GCM-SHA256 encrypted SMTP; 27 Apr 2016 16:36:24 -0000 Received: by mail-qk0-f194.google.com with SMTP id b63so3760942qkg.2 for ; Wed, 27 Apr 2016 09:36:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=C4/4LHaMl+t/8VbgzYjZYrfnU5GvKnOJMEd5JoH6mWg=; b=HjNum8f6Im4tvZverabLCcEO87GTLIHM8ARTruuUDUPJgFLVhSsbnAEiNbkNTo+Ash ccBmqzaCNFepfgshckOJd/Zw2pJF24f9TwxYT2seyRkQw6eEO7PEmrPDv54It+WF+DCK ljJlDjv2uBIjCk4zYaAwYgvdteTte++8KnHUBd5+t25Y355MtTxV65XLdurYFnwxYEfi hhhkwOaJqAmvxHH3mNvQBAu82RZol0GdRc39ePscqOLhY2fC2bgqPQQLonbxRZZVZYXj 7yeoBWKIAkCkWoO/4scvdMCId0iNMXE76E6IM36KD/WNjNDFat+BU+C+RiyNyBpIHTrE 9yPw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent; bh=C4/4LHaMl+t/8VbgzYjZYrfnU5GvKnOJMEd5JoH6mWg=; b=Qo0IilKprp7AAvS/1TTm0l5+ruzsyn96o9QwI6PU5zqViVfJG4M/s43aEHKTqcojkS EwdET9jkAxED5BfggzGUIMqkqMCfkaEXLb8RkqY4n9jTl2BMxI6rrT5emVA8CJr4rWco 1FKCxwFjSgiM63wily62BtaF7M0QTWvsidTcYz+CZS8EKAbF6TfdLuxMaLOZt6SVaakl 0HnKgGjDVJ/heZAXbXAuAMP5J9+kXd/MzGAE9H1fbCLc0qxvo1/4co9kwCIKdf4bfJ1F DFBrmirEuoZD5/QNsjIPKChVczZMniw18ORP1JGFjmIS4Ned4S9Sx+XPKiCGoFvpuag0 OpIg== X-Gm-Message-State: AOPr4FXzd5T3HLGjkrKcBWpiocYlaFeo028e9rcBXlqaHXnRjxhMptgOCFTuJyqFPt525Q== X-Received: by 10.55.133.193 with SMTP id h184mr9699498qkd.202.1461774983392; Wed, 27 Apr 2016 09:36:23 -0700 (PDT) Received: from x230.dumpdata.com (209-6-196-81.c3-0.smr-ubr2.sbo-smr.ma.cable.rcn.com. [209.6.196.81]) by smtp.gmail.com with ESMTPSA id p38sm1446639qgd.6.2016.04.27.09.36.21 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 27 Apr 2016 09:36:22 -0700 (PDT) Date: Wed, 27 Apr 2016 12:36:19 -0400 From: Konrad Rzeszutek Wilk To: Jan Beulich Message-ID: <20160427163618.GE26384@x230.dumpdata.com> References: <1461598514-5440-1-git-send-email-konrad.wilk@oracle.com> <1461598514-5440-25-git-send-email-konrad.wilk@oracle.com> <5720A21F02000078000E63C9@prv-mh.provo.novell.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5720A21F02000078000E63C9@prv-mh.provo.novell.com> User-Agent: Mutt/1.5.24 (2015-08-30) Cc: Keir Fraser , andrew.cooper3@citrix.com, mpohlack@amazon.de, ross.lagerwall@citrix.com, sasha.levin@oracle.com, xen-devel@lists.xenproject.org Subject: Re: [Xen-devel] [PATCH v9 24/27] xsplice: Stacking build-id dependency checking. 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-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Apr 27, 2016 at 03:27:27AM -0600, Jan Beulich wrote: > >>> On 25.04.16 at 17:35, wrote: > > @@ -25,7 +28,7 @@ clean:: > > .PHONY: config.h > > config.h: OLD_CODE_SZ=$(call CODE_SZ,$(BASEDIR)/xen-syms,xen_extra_version) > > config.h: NEW_CODE_SZ=$(call CODE_SZ,$<,xen_hello_world) > > -config.h: xen_hello_world_func.o > > +config.h: xen_hello_world_func.o xen_bye_world_func.o > > Why is that? I need OLD_CODE_SZ in xen_bye_world.c. Ah, right it should be the other way around - xen_bye_world.o depends on config.h And this change can be removed. > > > @@ -33,9 +36,43 @@ config.h: xen_hello_world_func.o > > xen_hello_world.o: xen_hello_world_func.o > > > > .PHONY: $(XSPLICE) > > -$(XSPLICE): config.h xen_hello_world_func.o xen_hello_world.o > > - $(LD) $(LDFLAGS) -r -o $(XSPLICE) xen_hello_world_func.o \ > > - xen_hello_world.o > > +$(XSPLICE): config.h xen_hello_world_func.o xen_hello_world.o note.o > > + $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(XSPLICE) \ > > + xen_hello_world_func.o xen_hello_world.o note.o > > Probably easier to read and maintain if you used $(filter %.o,$^) > here? > > > +xen_bye_world.o: xen_bye_world_func.o > > Again - why? B/c xen_bye_world.o depends on xen_bye_world_func.o ? Oh wait, they only depend during linking! It should be: xen_bye_world.o: config.h (and also for xen_hello_world.o case) > > > +.PHONY: $(XSPLICE_BYE) > > +$(XSPLICE_BYE): $(XSPLICE) config.h xen_bye_world_func.o xen_bye_world.o hello_world_note.o > > The object files depend on config.h, but the binary does only > indirectly via the object files I would guess. (This, just like the > question right above, would then apply to the $(XSPLICE) related > rules too, in an earlier patch.) xen_bye_world.c won't compile if config.h is not present. I need to make sure that config.h gets created before xen_bye_world.o gets built. And since config.h generation depends on the existence of xen_hello_world_func.o Is there a better way of making this dependency? > > > + $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(XSPLICE_BYE) \ > > + xen_bye_world_func.o xen_bye_world.o hello_world_note.o > > Same as above - better use $^ (and if config.h goes away as a > direct dependency, it looks like you don't even need $(filter ...)). I have to have config.h as dependency. If I do 'make -j1232131 tests' if I don't have config.h as dependency things eventually break. > > > +int xen_build_id_check(const Elf_Note *n, unsigned int n_sz, > > + const void **p, unsigned int *len) > > +{ > > + /* Check if we really have a build-id. */ > > + if ( NT_GNU_BUILD_ID != n->type ) > > + return -ENODATA; > > + > > + if ( n_sz <= sizeof(*n) ) > > + return -EINVAL; > > + > > + if ( n->namesz + n->descsz > UINT_MAX ) > > Afaict this is always false. I think you really want > > if ( n->namesz + n->descsz < n->namesz ) > > > + return -EINVAL; > > + > > + if ( n->namesz != 4 /* GNU\0 */) > > < 4 would suffice here (and be more flexible if odd padding gets > inserted by whatever generates the note) > > > + return -EINVAL; > > + > > + if ( n->namesz + n->descsz + sizeof(*n) > n_sz ) > > if ( n->namesz + n->descsz > n_sz - sizeof(*n) ) > > > @@ -98,18 +130,9 @@ static int __init xen_build_init(void) > > if ( &n[1] > __note_gnu_build_id_end ) > > return -ENODATA;; > > > > - /* Check if we really have a build-id. */ > > - if ( NT_GNU_BUILD_ID != n->type ) > > - return -ENODATA; > > + sz = (size_t)__note_gnu_build_id_end - (size_t)n; > > So let's hope sizeof(void *) == sizeof(size_t) (or else this would yield > compiler warnings). Hmm, so far no warnings on ARM32, ARM64 nor x86. But I will change the cast to (void *) so it is just pointer arithmetic. New patch: From a0bb72ff1723a320fb02c158f63d94b2a811a238 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Wed, 27 Apr 2016 12:26:50 -0400 Subject: [PATCH] xsplice: Stacking build-id dependency checking. We now expect that the ELF payloads be built with the --build-id. Also the .xsplice.deps section has to have the contents of the hypervisor (or a preceding payload) build-id. We already have the code to verify the Elf_Note build-id so export parts of it. This dependency means the hypervisor MUST be compiled with --build-id - so we gate the build of xSplice on the availability of said functionality. This does not impact the ordering of how the payloads can be loaded, but it does enforce an STRICT ordering when the payloads are applied. Also the REPLACE is special - we need to check that its dependency against the hypervisor - not the last applied patch. To make this easier to test we also add an extra test-case to be used - which can only be applied on top of the xen_hello_world payload. As in, one can apply xen_hello_world and then xen_bye_world on top of that. Not the other way. We also print the dependency and payloads build_in the keyhandler. Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: Andrew Cooper --- Cc: Keir Fraser Cc: Jan Beulich Cc: Andrew Cooper v3: First time included. v4: Andrew fix against the build_id.o mutilations. Andrew fix to not include extra symbols in binary.id v5: s/ssize_t/unsigned int/ v6: s/an NT_GNU../a NT_GNU/ - Squash "xsplice: Print dependency and payloads build_id in the keyhandler" in this patch. - Add in xen_build_id_check size of section for better checking. v7: Added Andrew's reviewed-by. Change the .name in test-case to adhere to spec. Dropped NT_GNU_BUILD_ID and moved that to earlier patch (build_id: Provide ld-embedded build-ids) Amended spec and code to only have one of .xsplice.depends and .note.gnu.build-id Expanded comment about note.o and why we don't use arch/x86/note.o.bin Moved xen_build_id_check definition to xsplice.h from version.h (and dropping the #include's in version.h) Sort header files in tests. v8: - Change two of the dprinkt from XENLOG_DEBUG to XENLOG_ERR v9: - Dropped the (unsigned long) casts since we use void. - Make the .xsplice_depends and .note.gnu_build_id be #defines. - Make the build section use $(XSPLICE_BYE) - Make the testcase include - Made comparisons on descsz and namesz a bit different (overflow checks, against value of 4, and against size) v10 - inline patches to response to v9: - Use filter() to only link .o files. - Make dependency for xen_bye_world.o be config.h only - Make overflow check for n->namesz and n->descsz be proper - Check n->namesz against less than 4. - Change check against header of Elf_Note - Calculate size (in bytes) of Elf_Note using pointer arithmetic. --- .gitignore | 1 + Config.mk | 1 + docs/misc/xsplice.markdown | 99 ++++++++++++++++++---------- xen/arch/x86/test/Makefile | 46 +++++++++++-- xen/arch/x86/test/xen_bye_world.c | 34 ++++++++++ xen/arch/x86/test/xen_bye_world_func.c | 22 +++++++ xen/common/Kconfig | 6 +- xen/common/version.c | 45 +++++++++---- xen/common/xsplice.c | 117 ++++++++++++++++++++++++++++++++- xen/include/xen/xsplice.h | 4 ++ 10 files changed, 323 insertions(+), 52 deletions(-) create mode 100644 xen/arch/x86/test/xen_bye_world.c create mode 100644 xen/arch/x86/test/xen_bye_world_func.c diff --git a/.gitignore b/.gitignore index 4a81f43..88cec1d 100644 --- a/.gitignore +++ b/.gitignore @@ -248,6 +248,7 @@ xen/arch/x86/efi/disabled xen/arch/x86/efi/mkreloc xen/arch/x86/test/config.h xen/arch/x86/test/xen_hello_world.xsplice +xen/arch/x86/test/xen_bye_world.xsplice xen/arch/*/efi/boot.c xen/arch/*/efi/compat.c xen/arch/*/efi/efi.h diff --git a/Config.mk b/Config.mk index 41f8c44..614dc9e 100644 --- a/Config.mk +++ b/Config.mk @@ -134,6 +134,7 @@ ifeq ($(call ld-ver-build-id,$(LD)),n) build_id_linker := else CFLAGS += -DBUILD_ID +export XEN_HAS_BUILD_ID=y build_id_linker := --build-id=sha1 endif diff --git a/docs/misc/xsplice.markdown b/docs/misc/xsplice.markdown index 35ebc28..4a98be1 100644 --- a/docs/misc/xsplice.markdown +++ b/docs/misc/xsplice.markdown @@ -283,8 +283,17 @@ The xSplice core code loads the payload as a standard ELF binary, relocates it and handles the architecture-specifc sections as needed. This process is much like what the Linux kernel module loader does. -The payload contains a section (xsplice_patch_func) with an array of structures -describing the functions to be patched: +The payload contains at least three sections: + + * `.xsplice.funcs` - which is an array of xsplice_patch_func structures. + * `.xsplice.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. + +### .xsplice.funcs + +The `.xsplice.funcs` contains an array of xsplice_patch_func structures +which describe the functions to be patched: 
 struct xsplice_patch_func {  
@@ -368,6 +377,23 @@ struct xsplice_patch_func xsplice_hello_world = {
 
 Code must be compiled with -fPIC.
 
+### .xsplice.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 `.xsplice.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.
+
+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
+checksum, MD5 checksum or any unique value.
+
+The size of these structures varies with the --build-id linker option.
+
 ## Hypercalls
 
 We will employ the sub operations of the system management hypercall (sysctl).
@@ -853,6 +879,42 @@ This is implemented in the Xen Project hypervisor.
 
 Only the privileged domain should be allowed to do this operation.
 
+### xSplice interdependencies
+
+xSplice patches interdependencies are tricky.
+
+There are the ways this can be addressed:
+ * A single large patch that subsumes and replaces all previous ones.
+   Over the life-time of patching the hypervisor this large patch
+   grows to accumulate all the code changes.
+ * Hotpatch stack - where an mechanism exists that loads the hotpatches
+   in the same order they were built in. We would need an build-id
+   of the hypevisor to make sure the hot-patches are build against the
+   correct build.
+ * Payload containing the old code to check against that. That allows
+   the hotpatches to be loaded indepedently (if they don't overlap) - or
+   if the old code also containst previously patched code - even if they
+   overlap.
+
+The disadvantage of the first large patch is that it can grow over
+time and not provide an bisection mechanism to identify faulty patches.
+
+The hot-patch stack puts stricts requirements on the order of the patches
+being loaded and requires an hypervisor build-id to match against.
+
+The old code allows much more flexibility and an additional guard,
+but is more complex to implement.
+
+The second option which requires an build-id of the hypervisor
+is implemented in the Xen Project hypervisor.
+
+Specifically each payload has two build-id ELF notes:
+ * The build-id of the payload itself (generated via --build-id).
+ * 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.
 
 # Not Yet Done
 
@@ -872,13 +934,6 @@ The implementation must also have a mechanism for (in no particular order):
  * NOP out the code sequence if `new_size` is zero.
  * Deal with other relocation types:  R_X86_64_[8,16,32,32S], R_X86_64_PC[8,16,64]
    in payload file.
- * An dependency mechanism for the payloads. To use that information to load:
-    - The appropiate payload. To verify that payload is built against the
-      hypervisor. This can be done via the `build-id`
-      or via providing an copy of the old code - so that the hypervisor can
-       verify it against the code in memory.
-    - To construct an appropiate order of payloads to load in case they
-      depend on each other.
 
 ### Handle inlined __LINE__
 
@@ -943,32 +998,6 @@ the function itself.
 Similar considerations are true to a lesser extent for __FILE__, but it
 could be argued that file renaming should be done outside of hotpatches.
 
-### xSplice interdependencies
-
-xSplice patches interdependencies are tricky.
-
-There are the ways this can be addressed:
- * A single large patch that subsumes and replaces all previous ones.
-   Over the life-time of patching the hypervisor this large patch
-   grows to accumulate all the code changes.
- * Hotpatch stack - where an mechanism exists that loads the hotpatches
-   in the same order they were built in. We would need an build-id
-   of the hypevisor to make sure the hot-patches are build against the
-   correct build.
- * Payload containing the old code to check against that. That allows
-   the hotpatches to be loaded indepedently (if they don't overlap) - or
-   if the old code also containst previously patched code - even if they
-   overlap.
-
-The disadvantage of the first large patch is that it can grow over
-time and not provide an bisection mechanism to identify faulty patches.
-
-The hot-patch stack puts stricts requirements on the order of the patches
-being loaded and requires an hypervisor build-id to match against.
-
-The old code allows much more flexibility and an additional guard,
-but is more complex to implement.
-
 ## Signature checking requirements.
 
 The signature checking requires that the layout of the data in memory
diff --git a/xen/arch/x86/test/Makefile b/xen/arch/x86/test/Makefile
index 364408d..349c603 100644
--- a/xen/arch/x86/test/Makefile
+++ b/xen/arch/x86/test/Makefile
@@ -6,17 +6,20 @@ CODE_SZ=$(shell nm --defined -S $(1) | grep $(2) | awk '{ print "0x"$$2}')
 .PHONY: default
 
 XSPLICE := xen_hello_world.xsplice
+XSPLICE_BYE := xen_bye_world.xsplice
 
 default: xsplice
 
 install: xsplice
 	$(INSTALL_DATA) $(XSPLICE) $(DESTDIR)$(DEBUG_DIR)/$(XSPLICE)
+	$(INSTALL_DATA) $(XSPLICE_BYE) $(DESTDIR)$(DEBUG_DIR)/$(XSPLICE_BYE)
 uninstall:
 	rm -f $(DESTDIR)$(DEBUG_DIR)/$(XSPLICE)
+	rm -f $(DESTDIR)$(DEBUG_DIR)/$(XSPLICE_BYE)
 
 .PHONY: clean
 clean::
-	rm -f *.o .*.o.d $(XSPLICE) config.h
+	rm -f *.o .*.o.d $(XSPLICE) $(XSPLICE_BYE) config.h *.bin
 
 #
 # To compute these values we need the binary files: xen-syms
@@ -33,8 +36,43 @@ config.h: xen_hello_world_func.o
 xen_hello_world.o: config.h
 
 .PHONY: $(XSPLICE)
-$(XSPLICE): config.h xen_hello_world_func.o xen_hello_world.o
-	$(LD) $(LDFLAGS) -r -o $(XSPLICE) $(filter %.o,$^)
+$(XSPLICE): config.h xen_hello_world_func.o xen_hello_world.o note.o
+	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(XSPLICE) \
+		$(filter %.o,$^)
+#
+# This target is only accessible if CONFIG_XSPLICE is defined, which
+# depends on $(build_id_linker) being available. Hence we do not
+# need any checks.
+#
+# N.B. The reason we don't use arch/x86/note.o is that it may
+# not be built (it is for EFI builds), and that we do not have
+# the note.o.bin to muck with (as it gets deleted)
+#
+.PHONY: note.o
+note.o:
+	$(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(BASEDIR)/xen-syms $@.bin
+	$(OBJCOPY) -I binary -O elf64-x86-64 -B i386:x86-64 \
+		   --rename-section=.data=.xsplice.depends -S $@.bin $@
+	rm -f $@.bin
+
+#
+# Extract the build-id of the xen_hello_world.xsplice
+# (which xen_bye_world will depend on).
+#
+.PHONY: hello_world_note.o
+hello_world_note.o: $(XSPLICE)
+	$(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(XSPLICE) $@.bin
+	$(OBJCOPY)  -I binary -O elf64-x86-64 -B i386:x86-64 \
+		   --rename-section=.data=.xsplice.depends -S $@.bin $@
+	rm -f $@.bin
+
+xen_bye_world.o: config.h
+
+.PHONY: $(XSPLICE_BYE)
+$(XSPLICE_BYE): $(XSPLICE) config.h xen_bye_world_func.o xen_bye_world.o hello_world_note.o
+	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(XSPLICE_BYE) \
+		$(filter %.o,$^)
+
 
 .PHONY: xsplice
-xsplice: $(XSPLICE)
+xsplice: $(XSPLICE) $(XSPLICE_BYE)
diff --git a/xen/arch/x86/test/xen_bye_world.c b/xen/arch/x86/test/xen_bye_world.c
new file mode 100644
index 0000000..f93f969
--- /dev/null
+++ b/xen/arch/x86/test/xen_bye_world.c
@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include "config.h"
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+static char bye_world_patch_this_fnc[] = "xen_extra_version";
+extern const char *xen_bye_world(void);
+
+struct xsplice_patch_func __section(".xsplice.funcs") xsplice_xen_bye_world = {
+    .version = XSPLICE_PAYLOAD_VERSION,
+    .name = bye_world_patch_this_fnc,
+    .new_addr = xen_bye_world,
+    .old_addr = xen_extra_version,
+    .new_size = NEW_CODE_SZ,
+    .old_size = OLD_CODE_SZ,
+};
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/test/xen_bye_world_func.c b/xen/arch/x86/test/xen_bye_world_func.c
new file mode 100644
index 0000000..32ef341
--- /dev/null
+++ b/xen/arch/x86/test/xen_bye_world_func.c
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include 
+
+/* Our replacement function for xen_hello_world. */
+const char *xen_bye_world(void)
+{
+    return "Bye World!";
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index e4f86c2..91ea904 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -60,6 +60,10 @@ config HAS_GDBSX
 config HAS_IOPORTS
 	bool
 
+config HAS_BUILD_ID
+	string
+	option env="XEN_HAS_BUILD_ID"
+
 # Enable/Disable kexec support
 config KEXEC
 	bool "kexec support"
@@ -192,7 +196,7 @@ endmenu
 config XSPLICE
 	bool "xSplice live patching support (TECH PREVIEW)"
 	default n
-	depends on X86
+	depends on X86 && HAS_BUILD_ID = "y"
 	---help---
 	  Allows a running Xen hypervisor to be dynamically patched using
 	  binary patches without rebooting. This is primarily used to binarily
diff --git a/xen/common/version.c b/xen/common/version.c
index 30578a6..0f96849 100644
--- a/xen/common/version.c
+++ b/xen/common/version.c
@@ -86,9 +86,41 @@ int xen_build_id(const void **p, unsigned int *len)
 /* Defined in linker script. */
 extern const Elf_Note __note_gnu_build_id_start[], __note_gnu_build_id_end[];
 
+int xen_build_id_check(const Elf_Note *n, unsigned int n_sz,
+                       const void **p, unsigned int *len)
+{
+    /* Check if we really have a build-id. */
+    if ( NT_GNU_BUILD_ID != n->type )
+        return -ENODATA;
+
+    if ( n_sz <= sizeof(*n) )
+        return -EINVAL;
+
+    if ( n->namesz + n->descsz < n->namesz )
+        return -EINVAL;
+
+    if ( n->namesz < 4 /* GNU\0 */)
+        return -EINVAL;
+
+    if ( n->namesz + n->descsz > n_sz - sizeof(*n) )
+        return -EINVAL;
+
+    /* Sanity check, name should be "GNU" for ld-generated build-id. */
+    if ( strncmp(ELFNOTE_NAME(n), "GNU", n->namesz) != 0 )
+        return -ENODATA;
+
+    if ( len )
+        *len = n->descsz;
+    if ( p )
+        *p = ELFNOTE_DESC(n);
+
+    return 0;
+}
+
 static int __init xen_build_init(void)
 {
     const Elf_Note *n = __note_gnu_build_id_start;
+    unsigned int sz;
 
     /* --build-id invoked with wrong parameters. */
     if ( __note_gnu_build_id_end <= &n[0] )
@@ -98,18 +130,9 @@ static int __init xen_build_init(void)
     if ( &n[1] > __note_gnu_build_id_end )
         return -ENODATA;;
 
-    /* Check if we really have a build-id. */
-    if ( NT_GNU_BUILD_ID != n->type )
-        return -ENODATA;
+    sz = (void *)__note_gnu_build_id_end - (void *)n;
 
-    /* Sanity check, name should be "GNU" for ld-generated build-id. */
-    if ( strncmp(ELFNOTE_NAME(n), "GNU", n->namesz) != 0 )
-        return -ENODATA;
-
-    build_id_len = n->descsz;
-    build_id_p = ELFNOTE_DESC(n);
-
-    return 0;
+    return xen_build_id_check(n, sz, &build_id_p, &build_id_len);
 }
 __initcall(xen_build_init);
 #endif
diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c
index 8243348..4e5c549 100644
--- a/xen/common/xsplice.c
+++ b/xen/common/xsplice.c
@@ -4,6 +4,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -42,6 +43,12 @@ static LIST_HEAD(applied_list);
 static unsigned int payload_cnt;
 static unsigned int payload_version = 1;
 
+/* To contain the ELF Note header. */
+struct xsplice_build_id {
+   const void *p;
+   unsigned int len;
+};
+
 struct payload {
     uint32_t state;                      /* One of the XSPLICE_STATE_*. */
     int32_t rc;                          /* 0 or -XEN_EXX. */
@@ -61,6 +68,8 @@ struct payload {
     struct virtual_region region;        /* symbol, bug.frame patching and
                                             exception table (x86). */
     unsigned int nsyms;                  /* Nr of entries in .strtab and symbols. */
+    struct xsplice_build_id id;          /* ELFNOTE_DESC(.note.gnu.build-id) of the payload. */
+    struct xsplice_build_id dep;         /* ELFNOTE_DESC(.xsplice.depends). */
     char name[XEN_XSPLICE_NAME_SIZE];    /* Name of it. */
 };
 
@@ -419,7 +428,9 @@ static int secure_payload(struct payload *payload, struct xsplice_elf *elf)
 static int check_special_sections(const struct xsplice_elf *elf)
 {
     unsigned int i;
-    static const char *const names[] = { ELF_XSPLICE_FUNC };
+    static const char *const names[] = { ELF_XSPLICE_FUNC,
+                                         ELF_XSPLICE_DEPENDS,
+                                         ELF_BUILD_ID_NOTE};
     DECLARE_BITMAP(found, ARRAY_SIZE(names)) = { 0 };
 
     for ( i = 0; i < ARRAY_SIZE(names); i++ )
@@ -459,6 +470,7 @@ static int prepare_payload(struct payload *payload,
     unsigned int i;
     struct xsplice_patch_func *f;
     struct virtual_region *region;
+    const Elf_Note *n;
 
     sec = xsplice_elf_sec_by_name(elf, ELF_XSPLICE_FUNC);
     ASSERT(sec);
@@ -515,6 +527,37 @@ static int prepare_payload(struct payload *payload,
         }
     }
 
+    sec = xsplice_elf_sec_by_name(elf, ELF_BUILD_ID_NOTE);
+    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->id.p, &payload->id.len) )
+            return -EINVAL;
+
+        if ( !payload->id.len || !payload->id.p )
+            return -EINVAL;
+    }
+
+    sec = xsplice_elf_sec_by_name(elf, ELF_XSPLICE_DEPENDS);
+    {
+        n = sec->load_addr;
+
+        if ( sec->sec->sh_size <= sizeof(*n) )
+            return -EINVAL;
+
+        if ( xen_build_id_check(n, sec->sec->sh_size,
+                                &payload->dep.p, &payload->dep.len) )
+            return -EINVAL;
+
+        if ( !payload->dep.len || !payload->dep.p )
+            return -EINVAL;
+    }
+
     /* Setup the virtual region with proper data. */
     region = &payload->region;
 
@@ -1244,6 +1287,55 @@ void check_for_xsplice_work(void)
     }
 }
 
+/*
+ * Only allow dependent payload is applied on top of the correct
+ * build-id.
+ *
+ * This enforces an stacking order - the first payload MUST be against the
+ * hypervisor. The second against the first payload, and so on.
+ *
+ * Unless the 'internal' parameter is used - in which case we only
+ * check against the hypervisor.
+ */
+static int build_id_dep(struct payload *payload, bool_t internal)
+{
+    const void *id = NULL;
+    unsigned int len = 0;
+    int rc;
+    const char *name = "hypervisor";
+
+    ASSERT(payload->dep.len && payload->dep.p);
+
+    /* First time user is against hypervisor. */
+    if ( internal )
+    {
+        rc = xen_build_id(&id, &len);
+        if ( rc )
+            return rc;
+    }
+    else
+    {
+        /* We should be against the last applied one. */
+        const struct payload *data;
+
+        data = list_last_entry(&applied_list, struct payload, applied_list);
+
+        id = data->id.p;
+        len = data->id.len;
+        name = data->name;
+    }
+
+    if ( payload->dep.len != len ||
+         memcmp(id, payload->dep.p, len) )
+    {
+        dprintk(XENLOG_ERR, "%s%s: check against %s build-id failed!\n",
+                XSPLICE, payload->name, name);
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
 static int xsplice_action(xen_sysctl_xsplice_action_t *action)
 {
     struct payload *data;
@@ -1283,6 +1375,18 @@ static int xsplice_action(xen_sysctl_xsplice_action_t *action)
     case XSPLICE_ACTION_REVERT:
         if ( data->state == XSPLICE_STATE_APPLIED )
         {
+            const struct payload *p;
+
+            p = list_last_entry(&applied_list, struct payload, applied_list);
+            ASSERT(p);
+            /* We should be the last applied one. */
+            if ( p != data )
+            {
+                dprintk(XENLOG_ERR, "%s%s: can't unload. Top is %s!\n",
+                        XSPLICE, data->name, p->name);
+                rc = -EBUSY;
+                break;
+            }
             data->rc = -EAGAIN;
             rc = schedule_work(data, action->cmd, action->timeout);
         }
@@ -1291,6 +1395,9 @@ static int xsplice_action(xen_sysctl_xsplice_action_t *action)
     case XSPLICE_ACTION_APPLY:
         if ( data->state == XSPLICE_STATE_CHECKED )
         {
+            rc = build_id_dep(data, !!list_empty(&applied_list));
+            if ( rc )
+                break;
             data->rc = -EAGAIN;
             rc = schedule_work(data, action->cmd, action->timeout);
         }
@@ -1299,6 +1406,9 @@ static int xsplice_action(xen_sysctl_xsplice_action_t *action)
     case XSPLICE_ACTION_REPLACE:
         if ( data->state == XSPLICE_STATE_CHECKED )
         {
+            rc = build_id_dep(data, 1 /* against hypervisor. */);
+            if ( rc )
+                break;
             data->rc = -EAGAIN;
             rc = schedule_work(data, action->cmd, action->timeout);
         }
@@ -1403,6 +1513,11 @@ static void xsplice_printall(unsigned char key)
                 }
             }
         }
+        if ( data->id.len )
+            printk("build-id=%*phN\n", data->id.len, data->id.p);
+
+        if ( data->dep.len )
+            printk("depend-on=%*phN\n", data->dep.len, data->dep.p);
     }
 
     spin_unlock(&payload_lock);
diff --git a/xen/include/xen/xsplice.h b/xen/include/xen/xsplice.h
index 849f58c..5ffb69d 100644
--- a/xen/include/xen/xsplice.h
+++ b/xen/include/xen/xsplice.h
@@ -28,6 +28,8 @@ struct xen_sysctl_xsplice_op;
 #define XSPLICE             "xsplice: "
 /* ELF payload special section names. */
 #define ELF_XSPLICE_FUNC    ".xsplice.funcs"
+#define ELF_XSPLICE_DEPENDS ".xsplice.depends"
+#define ELF_BUILD_ID_NOTE   ".note.gnu.build-id"
 
 struct xsplice_symbol {
     const char *name;
@@ -40,6 +42,8 @@ int xsplice_op(struct xen_sysctl_xsplice_op *);
 void check_for_xsplice_work(void);
 unsigned long xsplice_symbols_lookup_by_name(const char *symname);
 bool_t is_patch(const void *addr);
+int xen_build_id_check(const Elf_Note *n, unsigned int n_sz,
+                       const void **p, unsigned int *len);
 
 /* Arch hooks. */
 int arch_xsplice_verify_elf(const struct xsplice_elf *elf);