Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Pawel Wieczorkiewicz <wipawel@amazon.de>
To: <xen-devel@lists.xen.org>, <xen-devel@lists.xenproject.org>
Cc: wipawel@amazon.com, Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wl@xen.org>, Ross Lagerwall <ross.lagerwall@citrix.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	mpohlack@amazon.com, Tim Deegan <tim@xen.org>,
	Pawel Wieczorkiewicz <wipawel@amazon.de>,
	Julien Grall <julien.grall@arm.com>,
	Jan Beulich <jbeulich@suse.com>
Subject: [Xen-devel] [PATCH 10/14] livepatch: Add support for inline asm hotpatching expectations
Date: Wed, 21 Aug 2019 08:19:27 +0000
Message-ID: <20190821081931.90887-11-wipawel@amazon.de> (raw)
In-Reply-To: <20190821081931.90887-1-wipawel@amazon.de>

This is the initial implementation of the expectations enhancement
to improve inline asm hotpatching.

Expectations are designed as optional feature, since the main use of
them is planned for inline asm hotpatching. The flag enabled allows
to control the expectation state.
Each expectation has data and len fields that describe the data
that is expected to be found at a given patching (old_addr) location.
The len must not exceed the data array size. The data array size
follows the size of the opaque array, since the opaque array holds
the original data and therefore must match what is specified in the
expectation (if enabled).

The payload structure is modified as each expectation structure is
part of the livepatch_func structure and hence extends the payload.
The payload version is bumped to 3 with this change to highlight the
ABI modification and enforce proper support.

Each expectation is checked prior to the apply action (i.e. as late
as possible to check against the most current state of the code).

For the replace action a new payload's expectations are checked AFTER
all applied payloads are successfully reverted, but BEFORE new payload
is applied. That breaks the replace action's atomicity and in case of
an expectation check failure would leave a system with all payloads
reverted. That is obviously insecure. Use it with caution and act
upon replace errors!

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
Reviewed-by: Martin Pohlack <mpohlack@amazon.de>
Reviewed-by: Norbert Manthey <nmanthey@amazon.de>
---
 xen/common/livepatch.c      | 71 +++++++++++++++++++++++++++++++++++++++++++++
 xen/include/public/sysctl.h | 17 +++++++++--
 2 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 090a48977b..8aef2fd12e 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -560,6 +560,58 @@ static int check_patching_sections(const struct livepatch_elf *elf)
     return 0;
 }
 
+static inline int livepatch_verify_expectation_fn(const struct livepatch_func *func)
+{
+    const livepatch_expectation_t *exp = &func->expect;
+
+    /* Ignore disabled expectations. */
+    if ( !exp->enabled )
+        return 0;
+
+    /* There is nothing to expect */
+    if ( !func->old_addr )
+        return -EFAULT;
+
+    if ( exp->len > sizeof(exp->data))
+        return -EOVERFLOW;
+
+    /* Incorrect expectation */
+    if ( func->old_size < exp->len )
+        return -ERANGE;
+
+    if ( memcmp(func->old_addr, exp->data, exp->len) )
+    {
+        printk(XENLOG_ERR LIVEPATCH "%s: expectation failed: expected:%*phN, actual:%*phN\n",
+               func->name, exp->len, exp->data, exp->len, func->old_addr);
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+static inline int livepatch_check_expectations(const struct payload *payload)
+{
+    int i, rc;
+
+    printk(XENLOG_INFO LIVEPATCH "%s: Verifying enabled expectations for all functions\n",
+           payload->name);
+
+    for ( i = 0; i < payload->nfuncs; i++ )
+    {
+        const struct livepatch_func *func = &(payload->funcs[i]);
+
+        rc = livepatch_verify_expectation_fn(func);
+        if ( rc )
+        {
+            printk(XENLOG_ERR LIVEPATCH "%s: expectations of %s failed (rc=%d), aborting!\n",
+                   payload->name, func->name ?: "unknown", rc);
+            return rc;
+        }
+    }
+
+    return 0;
+}
+
 /*
  * Lookup specified section and when exists assign its address to a specified hook.
  * Perform section pointer and size validation: single hook sections must contain a
@@ -1347,6 +1399,20 @@ static void livepatch_do_action(void)
 
         if ( rc == 0 )
         {
+            /*
+             * Make sure all expectation requirements are met.
+             * Beware all the payloads are reverted at this point.
+             * If expectations are not met the system is left in a
+             * completely UNPATCHED state!
+             */
+            rc = livepatch_check_expectations(data);
+            if ( rc )
+            {
+                printk(XENLOG_ERR LIVEPATCH "%s: SYSTEM MIGHT BE INSECURE: "
+                       "Replace action has been aborted after reverting ALL payloads!\n", data->name);
+                break;
+            }
+
             if ( is_hook_enabled(data->hooks.apply.action) )
             {
                 printk(XENLOG_INFO LIVEPATCH "%s: Calling apply action hook function\n", data->name);
@@ -1800,6 +1866,11 @@ static int livepatch_action(struct xen_sysctl_livepatch_action *action)
                     break;
             }
 
+            /* Make sure all expectation requirements are met. */
+            rc = livepatch_check_expectations(data);
+            if ( rc )
+                break;
+
             if ( is_hook_enabled(data->hooks.apply.pre) )
             {
                 printk(XENLOG_INFO LIVEPATCH "%s: Calling pre-apply hook function\n", data->name);
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index b55ad6d050..e18322350d 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -818,7 +818,7 @@ struct xen_sysctl_cpu_featureset {
  *     If zero exit with success.
  */
 
-#define LIVEPATCH_PAYLOAD_VERSION 2
+#define LIVEPATCH_PAYLOAD_VERSION 3
 /*
  * .livepatch.funcs structure layout defined in the `Payload format`
  * section in the Live Patch design document.
@@ -826,6 +826,18 @@ struct xen_sysctl_cpu_featureset {
  * We guard this with __XEN__ as toolstacks SHOULD not use it.
  */
 #ifdef __XEN__
+#define LIVEPATCH_OPAQUE_SIZE 31
+
+struct livepatch_expectation {
+    uint8_t enabled : 1;
+    uint8_t len : 5;        /* Length of data up to LIVEPATCH_OPAQUE_SIZE
+                               (5 bits is enough for now) */
+    uint8_t data[LIVEPATCH_OPAQUE_SIZE]; /* Same size as opaque[] buffer of
+                                            struct livepatch_func. This is the
+                                            max number of bytes to be patched */
+};
+typedef struct livepatch_expectation livepatch_expectation_t;
+
 typedef enum livepatch_func_state {
     LIVEPATCH_FUNC_NOT_APPLIED = 0,
     LIVEPATCH_FUNC_APPLIED = 1
@@ -838,11 +850,12 @@ struct livepatch_func {
     uint32_t new_size;
     uint32_t old_size;
     uint8_t version;        /* MUST be LIVEPATCH_PAYLOAD_VERSION. */
-    uint8_t opaque[31];
+    uint8_t opaque[LIVEPATCH_OPAQUE_SIZE];
 #if defined CONFIG_X86
     uint8_t applied;
     uint8_t _pad[7];
 #endif
+    livepatch_expectation_t expect;
 };
 typedef struct livepatch_func livepatch_func_t;
 #endif
-- 
2.16.5




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  parent reply index

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-21  8:19 [Xen-devel] [PATCH 00/14] livepatch: new features and fixes Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 01/14] livepatch: Always check hypervisor build ID upon hotpatch upload Pawel Wieczorkiewicz
2019-08-21 18:16   ` Konrad Rzeszutek Wilk
2019-08-21  8:19 ` [Xen-devel] [PATCH 02/14] livepatch: Allow to override inter-modules buildid dependency Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 03/14] python: Add XC binding for Xen build ID Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 04/14] livepatch: Export payload structure via livepatch_payload.h Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 05/14] livepatch: Implement pre-|post- apply|revert hooks Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 06/14] livepatch: Add support for apply|revert action replacement hooks Pawel Wieczorkiewicz
2019-08-21 18:31   ` Konrad Rzeszutek Wilk
2019-08-21 19:06     ` Wieczorkiewicz, Pawel
2019-08-26 14:30       ` Konrad Rzeszutek Wilk
2019-08-21  8:19 ` [Xen-devel] [PATCH 07/14] livepatch: Do not enforce ELF_LIVEPATCH_FUNC section presence Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 08/14] livepatch: always print XENLOG_ERR information Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 09/14] livepatch: Add per-function applied/reverted state tracking marker Pawel Wieczorkiewicz
2019-08-21 18:28   ` Konrad Rzeszutek Wilk
2019-08-21 19:00     ` Wieczorkiewicz, Pawel
2019-08-21 21:34   ` Julien Grall
2019-08-22  7:44     ` Wieczorkiewicz, Pawel
2019-08-22 10:07       ` Julien Grall
2019-08-22 10:20         ` Wieczorkiewicz, Pawel
2019-08-22 10:43           ` Julien Grall
2019-08-22 11:15             ` Wieczorkiewicz, Pawel
2019-08-22 15:02               ` Julien Grall
2019-08-22 10:29   ` Julien Grall
2019-08-22 11:02     ` Wieczorkiewicz, Pawel
2019-08-22 15:30       ` Julien Grall
2019-08-22 15:42         ` Wieczorkiewicz, Pawel
2019-08-21  8:19 ` Pawel Wieczorkiewicz [this message]
2019-08-21 18:30   ` [Xen-devel] [PATCH 10/14] livepatch: Add support for inline asm hotpatching expectations Konrad Rzeszutek Wilk
2019-08-21 19:02     ` Wieczorkiewicz, Pawel
2019-08-22 10:31   ` Julien Grall
2019-08-22 11:03     ` Wieczorkiewicz, Pawel
2019-08-21  8:19 ` [Xen-devel] [PATCH 11/14] livepatch: Add support for modules .modinfo section metadata Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 12/14] livepatch: Handle arbitrary size names with the list operation Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 13/14] livepatch: Add metadata runtime retrieval mechanism Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 14/14] livepatch: Add python bindings for livepatch operations Pawel Wieczorkiewicz
2019-08-22 21:55   ` Marek Marczykowski-Górecki
2019-08-27  8:46 ` [Xen-devel] [PATCH v2 00/12] livepatch: new features and fixes Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 01/12] livepatch: Always check hypervisor build ID upon hotpatch upload Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 02/12] livepatch: Allow to override inter-modules buildid dependency Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 03/12] livepatch: Export payload structure via livepatch_payload.h Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 04/12] livepatch: Implement pre-|post- apply|revert hooks Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 05/12] livepatch: Add support for apply|revert action replacement hooks Pawel Wieczorkiewicz
2019-08-27 16:58     ` Konrad Rzeszutek Wilk
2019-08-28  7:37       ` Wieczorkiewicz, Pawel
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 06/12] livepatch: Do not enforce ELF_LIVEPATCH_FUNC section presence Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 07/12] livepatch: Add per-function applied/reverted state tracking marker Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 08/12] livepatch: Add support for inline asm hotpatching expectations Pawel Wieczorkiewicz
2019-08-29 14:34     ` Konrad Rzeszutek Wilk
2019-08-29 15:29       ` Wieczorkiewicz, Pawel
2019-08-29 15:58     ` Konrad Rzeszutek Wilk
2019-08-29 16:16       ` Wieczorkiewicz, Pawel
2019-08-29 17:49         ` Konrad Rzeszutek Wilk
2019-08-29 19:07           ` Wieczorkiewicz, Pawel
2019-08-29 20:48             ` Konrad Rzeszutek Wilk
2019-09-05 18:05     ` Konrad Rzeszutek Wilk
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 09/12] livepatch: Add support for modules .modinfo section metadata Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 10/12] livepatch: Handle arbitrary size names with the list operation Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 11/12] livepatch: Add metadata runtime retrieval mechanism Pawel Wieczorkiewicz
2019-08-29 20:48     ` Konrad Rzeszutek Wilk
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 12/12] livepatch: Add python bindings for livepatch operations Pawel Wieczorkiewicz
2019-08-28 13:21     ` Marek Marczykowski-Górecki
2019-08-29 19:23   ` [Xen-devel] [PATCH v2 00/12] livepatch: new features and fixes Konrad Rzeszutek Wilk
2019-09-05 19:13   ` Konrad Rzeszutek Wilk
2019-09-06 22:52     ` Julien Grall
2019-09-06 22:42   ` Julien Grall

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190821081931.90887-11-wipawel@amazon.de \
    --to=wipawel@amazon.de \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=julien.grall@arm.com \
    --cc=konrad.wilk@oracle.com \
    --cc=mpohlack@amazon.com \
    --cc=ross.lagerwall@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=tim@xen.org \
    --cc=wipawel@amazon.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git