From patchwork Mon Sep 16 10:59:38 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Wieczorkiewicz, Pawel" X-Patchwork-Id: 11146793 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 AB57B14ED for ; Mon, 16 Sep 2019 11:05:15 +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 786C6214AF for ; Mon, 16 Sep 2019 11:05:15 +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="F9nuQgz1" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 786C6214AF 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 1i9ons-0005py-Pz; Mon, 16 Sep 2019 11:04:20 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1i9onr-0005nE-7v for xen-devel@lists.xenproject.org; Mon, 16 Sep 2019 11:04:19 +0000 X-Inumbo-ID: b44a5a58-d871-11e9-95df-12813bfff9fa Received: from smtp-fw-6001.amazon.com (unknown [52.95.48.154]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id b44a5a58-d871-11e9-95df-12813bfff9fa; Mon, 16 Sep 2019 11:04:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.de; i=@amazon.de; q=dns/txt; s=amazon201209; t=1568631847; x=1600167847; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version; bh=bGGJkcyFYglxgMtxtDcIzGxVHjQCxqmTyuDEsH/GoYY=; b=F9nuQgz1BBOwgwtxzkCRwyHwfzpvZJ8nwh3CJNkSdoJaCI9Dh+mosWNE E24i04U8gy5BDOcXHPz64t8idZRxF0Eyyd8Pk6B/y+N4wHYtGbw/6lUiu 0CFL18cjUbkIPbVRiUkwJvjePLLbmbk8XCVcfj+/b5cFik8pQ+f9dzF30 c=; X-IronPort-AV: E=Sophos;i="5.64,512,1559520000"; d="scan'208";a="415446497" Received: from iad6-co-svc-p1-lb1-vlan3.amazon.com (HELO email-inbound-relay-1a-16acd5e0.us-east-1.amazon.com) ([10.124.125.6]) by smtp-border-fw-out-6001.iad6.amazon.com with ESMTP; 16 Sep 2019 11:04:06 +0000 Received: from EX13MTAUEA001.ant.amazon.com (iad55-ws-svc-p15-lb9-vlan2.iad.amazon.com [10.40.159.162]) by email-inbound-relay-1a-16acd5e0.us-east-1.amazon.com (Postfix) with ESMTPS id C7587A281E; Mon, 16 Sep 2019 11:04:03 +0000 (UTC) Received: from EX13D05EUC002.ant.amazon.com (10.43.164.231) by EX13MTAUEA001.ant.amazon.com (10.43.61.243) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Mon, 16 Sep 2019 11:03:44 +0000 Received: from EX13MTAUEB001.ant.amazon.com (10.43.60.96) by EX13D05EUC002.ant.amazon.com (10.43.164.231) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Mon, 16 Sep 2019 11:03:41 +0000 Received: from dev-dsk-wipawel-1a-0c4e6d58.eu-west-1.amazon.com (10.4.134.33) by mail-relay.amazon.com (10.43.60.129) with Microsoft SMTP Server id 15.0.1367.3 via Frontend Transport; Mon, 16 Sep 2019 11:03:39 +0000 From: Pawel Wieczorkiewicz To: , Date: Mon, 16 Sep 2019 10:59:38 +0000 Message-ID: <20190916105945.93632-6-wipawel@amazon.de> X-Mailer: git-send-email 2.16.5 In-Reply-To: <20190916105945.93632-1-wipawel@amazon.de> References: <20190916105945.93632-1-wipawel@amazon.de> MIME-Version: 1.0 Precedence: Bulk Subject: [Xen-devel] [PATCH v3 05/12] livepatch: Add support for apply|revert action replacement hooks 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" By default, in the quiescing zone, a hotpatch payload is applied with apply_payload() and reverted with revert_payload() functions. Both of the functions receive the payload struct pointer as a parameter. The functions are also a place where standard 'load' and 'unload' module hooks are executed. To increase hotpatching system's agility and provide more flexiable long-term hotpatch solution, allow to overwrite the default apply and revert action functions with hook-like supplied alternatives. The alternative functions are optional and the default functions are used by default. Since the alternative functions have direct access to the hotpatch payload structure, they can better control context of the 'load' and 'unload' hooks execution as well as exact instructions replacement workflows. They can be also easily extended to support extra features in the future. To simplify the alternative function generation move code responsible for payload and hotpatch region registration outside of the function. That way it is guaranteed that the registration step occurs even for newly supplied functions. Signed-off-by: Pawel Wieczorkiewicz Reviewed-by: Petre Eftime Reviewed-by: Martin Pohlack Reviewed-by: Norbert Manthey Reviewed-by: Andra-Irina Paraschiv Reviewed-by: Bjoern Doebel Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: Ross Lagerwall --- Changed since v1: * added corresponding documentation * added tests docs/misc/livepatch.pandoc | 23 ++++++++ xen/common/livepatch.c | 66 ++++++++++++++++++---- xen/include/xen/livepatch_payload.h | 10 ++++ xen/test/livepatch/Makefile | 10 +++- xen/test/livepatch/xen_action_hooks.c | 100 ++++++++++++++++++++++++++++++++++ 5 files changed, 198 insertions(+), 11 deletions(-) create mode 100644 xen/test/livepatch/xen_action_hooks.c diff --git a/docs/misc/livepatch.pandoc b/docs/misc/livepatch.pandoc index ae8566eb30..6fafb9e4b1 100644 --- a/docs/misc/livepatch.pandoc +++ b/docs/misc/livepatch.pandoc @@ -275,6 +275,7 @@ The payload contains at least three sections: * `.livepatch.funcs` - which is an array of livepatch_func structures. and/or any of: * `.livepatch.hooks.{preapply,postapply,prerevert,postrevert}' + * `.livepatch.hooks.{apply,revert}` - which are a pointer to a hook function pointer. * `.livepatch.xen_depends` - which is an ELF Note that describes what Xen @@ -356,6 +357,14 @@ met. * `.livepatch.hooks.{prerevert,postrevert}` - which are a pointer to a single hook function pointer. +Finally, it optionally may also contain the address of apply or revert action +hooks to be called instead of the default apply and revert payload actions +(while all CPUs are kept in quiescing zone). These hooks do have access to +payload structure. + + * `.livepatch.hooks.{apply,revert}` + - which are a pointer to a single hook function pointer. + ### Example of .livepatch.funcs A simple example of what a payload file can be: @@ -469,6 +478,20 @@ The type definition of the function are as follow: typedef void livepatch_postcall_t(livepatch_payload_t *arg); +#### .livepatch.hooks.apply and .livepatch.hooks.revert + +This section contains a pointer to a single function pointer to be executed +instead of a default apply (or revert) action function. This is useful to +replace or augment default behavior of the apply (or revert) action that +requires all CPUs to be in the quiescing zone. +This type of hooks do have access to payload structure. + +Each entry in this array is eight bytes. + +The type definition of the function are as follow: + + typedef int livepatch_actioncall_t(livepatch_payload_t *arg); + ### .livepatch.xen_depends, .livepatch.depends and .note.gnu.build-id To support dependencies checking and safe loading (to load the diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index c5dae8814f..577b926ba4 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -587,8 +587,11 @@ static int prepare_payload(struct payload *payload, LIVEPATCH_ASSIGN_MULTI_HOOK(elf, payload->unload_funcs, payload->n_unload_funcs, ".livepatch.hooks.unload"); LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.apply.pre, ".livepatch.hooks.preapply"); + LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.apply.action, ".livepatch.hooks.apply"); LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.apply.post, ".livepatch.hooks.postapply"); + LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.revert.pre, ".livepatch.hooks.prerevert"); + LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.revert.action, ".livepatch.hooks.revert"); LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.revert.post, ".livepatch.hooks.postrevert"); sec = livepatch_elf_sec_by_name(elf, ELF_BUILD_ID_NOTE); @@ -1114,6 +1117,11 @@ static int apply_payload(struct payload *data) arch_livepatch_revive(); + return 0; +} + +static inline void apply_payload_tail(struct payload *data) +{ /* * We need RCU variant (which has barriers) in case we crash here. * The applied_list is iterated by the trap code. @@ -1121,7 +1129,7 @@ static int apply_payload(struct payload *data) list_add_tail_rcu(&data->applied_list, &applied_list); register_virtual_region(&data->region); - return 0; + data->state = LIVEPATCH_STATE_APPLIED; } static int revert_payload(struct payload *data) @@ -1154,6 +1162,11 @@ static int revert_payload(struct payload *data) ASSERT(!local_irq_is_enabled()); arch_livepatch_revive(); + return 0; +} + +static inline void revert_payload_tail(struct payload *data) +{ /* * We need RCU variant (which has barriers) in case we crash here. @@ -1163,7 +1176,7 @@ static int revert_payload(struct payload *data) unregister_virtual_region(&data->region); data->reverted = true; - return 0; + data->state = LIVEPATCH_STATE_CHECKED; } /* @@ -1183,15 +1196,31 @@ static void livepatch_do_action(void) switch ( livepatch_work.cmd ) { case LIVEPATCH_ACTION_APPLY: - rc = apply_payload(data); + if ( is_hook_enabled(data->hooks.apply.action) ) + { + printk(XENLOG_INFO LIVEPATCH "%s: Calling apply action hook function\n", data->name); + + rc = (*data->hooks.apply.action)(data); + } + else + rc = apply_payload(data); + if ( rc == 0 ) - data->state = LIVEPATCH_STATE_APPLIED; + apply_payload_tail(data); break; case LIVEPATCH_ACTION_REVERT: - rc = revert_payload(data); + if ( is_hook_enabled(data->hooks.revert.action) ) + { + printk(XENLOG_INFO LIVEPATCH "%s: Calling revert action hook function\n", data->name); + + rc = (*data->hooks.revert.action)(data); + } + else + rc = revert_payload(data); + if ( rc == 0 ) - data->state = LIVEPATCH_STATE_CHECKED; + revert_payload_tail(data); break; case LIVEPATCH_ACTION_REPLACE: @@ -1202,9 +1231,18 @@ static void livepatch_do_action(void) */ list_for_each_entry_safe_reverse ( other, tmp, &applied_list, applied_list ) { - other->rc = revert_payload(other); + if ( is_hook_enabled(other->hooks.revert.action) ) + { + printk(XENLOG_INFO LIVEPATCH "%s: Calling revert action hook function\n", other->name); + + other->rc = (*other->hooks.revert.action)(other); + } + else + other->rc = revert_payload(other); + + if ( other->rc == 0 ) - other->state = LIVEPATCH_STATE_CHECKED; + revert_payload_tail(other); else { rc = -EINVAL; @@ -1214,9 +1252,17 @@ static void livepatch_do_action(void) if ( rc == 0 ) { - rc = apply_payload(data); + if ( is_hook_enabled(data->hooks.apply.action) ) + { + printk(XENLOG_INFO LIVEPATCH "%s: Calling apply action hook function\n", data->name); + + rc = (*data->hooks.apply.action)(data); + } + else + rc = apply_payload(data); + if ( rc == 0 ) - data->state = LIVEPATCH_STATE_APPLIED; + apply_payload_tail(data); } break; diff --git a/xen/include/xen/livepatch_payload.h b/xen/include/xen/livepatch_payload.h index cd20944cc4..ff16af0dd6 100644 --- a/xen/include/xen/livepatch_payload.h +++ b/xen/include/xen/livepatch_payload.h @@ -22,11 +22,13 @@ typedef void livepatch_loadcall_t(void); typedef void livepatch_unloadcall_t(void); typedef int livepatch_precall_t(livepatch_payload_t *arg); +typedef int livepatch_actioncall_t(livepatch_payload_t *arg); typedef void livepatch_postcall_t(livepatch_payload_t *arg); struct livepatch_hooks { struct { livepatch_precall_t *const *pre; + livepatch_actioncall_t *const *action; livepatch_postcall_t *const *post; } apply, revert; }; @@ -91,6 +93,10 @@ struct payload { livepatch_precall_t *__attribute__((weak, used)) \ const livepatch_preapply_data_##_fn __section(".livepatch.hooks.preapply") = _fn; +#define LIVEPATCH_APPLY_HOOK(_fn) \ + livepatch_actioncall_t *__attribute__((weak, used)) \ + const livepatch_apply_data_##_fn __section(".livepatch.hooks.apply") = _fn; + #define LIVEPATCH_POSTAPPLY_HOOK(_fn) \ livepatch_postcall_t *__attribute__((weak, used)) \ const livepatch_postapply_data_##_fn __section(".livepatch.hooks.postapply") = _fn; @@ -99,6 +105,10 @@ struct payload { livepatch_precall_t *__attribute__((weak, used)) \ const livepatch_prerevert_data_##_fn __section(".livepatch.hooks.prerevert") = _fn; +#define LIVEPATCH_REVERT_HOOK(_fn) \ + livepatch_actioncall_t *__attribute__((weak, used)) \ + const livepatch_revert_data_##_fn __section(".livepatch.hooks.revert") = _fn; + #define LIVEPATCH_POSTREVERT_HOOK(_fn) \ livepatch_postcall_t *__attribute__((weak, used)) \ const livepatch_postrevert_data_##_fn __section(".livepatch.hooks.postrevert") = _fn; diff --git a/xen/test/livepatch/Makefile b/xen/test/livepatch/Makefile index a94bc48536..116e52e774 100644 --- a/xen/test/livepatch/Makefile +++ b/xen/test/livepatch/Makefile @@ -22,6 +22,7 @@ LIVEPATCH_NOP := xen_nop.livepatch LIVEPATCH_NO_XEN_BUILDID := xen_no_xen_buildid.livepatch LIVEPATCH_PREPOST_HOOKS := xen_prepost_hooks.livepatch LIVEPATCH_PREPOST_HOOKS_FAIL := xen_prepost_hooks_fail.livepatch +LIVEPATCH_ACTION_HOOKS := xen_action_hooks.livepatch LIVEPATCHES += $(LIVEPATCH) LIVEPATCHES += $(LIVEPATCH_BYE) @@ -30,6 +31,7 @@ LIVEPATCHES += $(LIVEPATCH_NOP) LIVEPATCHES += $(LIVEPATCH_NO_XEN_BUILDID) LIVEPATCHES += $(LIVEPATCH_PREPOST_HOOKS) LIVEPATCHES += $(LIVEPATCH_PREPOST_HOOKS_FAIL) +LIVEPATCHES += $(LIVEPATCH_ACTION_HOOKS) LIVEPATCH_DEBUG_DIR ?= $(DEBUG_DIR)/xen-livepatch @@ -144,6 +146,12 @@ xen_prepost_hooks_fail.o: config.h $(LIVEPATCH_PREPOST_HOOKS_FAIL): xen_prepost_hooks_fail.o xen_hello_world_func.o note.o xen_note.o $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_PREPOST_HOOKS_FAIL) $^ +xen_actions_hooks.o: config.h + +.PHONY: $(LIVEPATCH_ACTION_HOOKS) +$(LIVEPATCH_ACTION_HOOKS): xen_action_hooks.o xen_hello_world_func.o note.o xen_note.o + $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_ACTION_HOOKS) $^ + .PHONY: livepatch livepatch: $(LIVEPATCH) $(LIVEPATCH_BYE) $(LIVEPATCH_REPLACE) $(LIVEPATCH_NOP) $(LIVEPATCH_NO_XEN_BUILDID) \ - $(LIVEPATCH_PREPOST_HOOKS) $(LIVEPATCH_PREPOST_HOOKS_FAIL) + $(LIVEPATCH_PREPOST_HOOKS) $(LIVEPATCH_PREPOST_HOOKS_FAIL) $(LIVEPATCH_ACTION_HOOKS) diff --git a/xen/test/livepatch/xen_action_hooks.c b/xen/test/livepatch/xen_action_hooks.c new file mode 100644 index 0000000000..a947afc41f --- /dev/null +++ b/xen/test/livepatch/xen_action_hooks.c @@ -0,0 +1,100 @@ +/* + * Copyright (c) 2019 Amazon.com, Inc. or its affiliates. All rights reserved. + * + */ + +#include "config.h" +#include +#include +#include +#include +#include + +#include + +static const char hello_world_patch_this_fnc[] = "xen_extra_version"; +extern const char *xen_hello_world(void); + +static unsigned int apply_cnt; +static unsigned int revert_cnt; + +static int apply_hook(livepatch_payload_t *payload) +{ + int i; + + printk(KERN_DEBUG "%s: Hook starting.\n", __func__); + + for (i = 0; i < payload->nfuncs; i++) + { + struct livepatch_func *func = &payload->funcs[i]; + + apply_cnt++; + + printk(KERN_DEBUG "%s: applying: %s\n", __func__, func->name); + } + + printk(KERN_DEBUG "%s: Hook done.\n", __func__); + + return 0; +} + +static int revert_hook(livepatch_payload_t *payload) +{ + int i; + + printk(KERN_DEBUG "%s: Hook starting.\n", __func__); + + for (i = 0; i < payload->nfuncs; i++) + { + struct livepatch_func *func = &payload->funcs[i]; + + revert_cnt++; + + printk(KERN_DEBUG "%s: reverting: %s\n", __func__, func->name); + } + + printk(KERN_DEBUG "%s: Hook done.\n", __func__); + + return 0; +} + +static void post_revert_hook(livepatch_payload_t *payload) +{ + int i; + + printk(KERN_DEBUG "%s: Hook starting.\n", __func__); + + for (i = 0; i < payload->nfuncs; i++) + { + struct livepatch_func *func = &payload->funcs[i]; + + printk(KERN_DEBUG "%s: reverted: %s\n", __func__, func->name); + } + + BUG_ON(apply_cnt != 1 || revert_cnt != 1); + printk(KERN_DEBUG "%s: Hook done.\n", __func__); +} + +LIVEPATCH_APPLY_HOOK(apply_hook); +LIVEPATCH_REVERT_HOOK(revert_hook); + +LIVEPATCH_POSTREVERT_HOOK(post_revert_hook); + +struct livepatch_func __section(".livepatch.funcs") livepatch_xen_hello_world = { + .version = LIVEPATCH_PAYLOAD_VERSION, + .name = hello_world_patch_this_fnc, + .new_addr = xen_hello_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: + */