From patchwork Sat Sep 28 15:12:58 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Wieczorkiewicz, Pawel" X-Patchwork-Id: 11165443 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 9BF61912 for ; Sat, 28 Sep 2019 15:18:29 +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 6BDC320880 for ; Sat, 28 Sep 2019 15:18:29 +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="MEPFdmXE" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6BDC320880 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 1iEETC-0001ZN-Ts; Sat, 28 Sep 2019 15:17:14 +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 1iEETA-0001Yq-RV for xen-devel@lists.xenproject.org; Sat, 28 Sep 2019 15:17:12 +0000 X-Inumbo-ID: 0b0ca61c-e203-11e9-969c-12813bfff9fa Received: from smtp-fw-33001.amazon.com (unknown [207.171.190.10]) by localhost (Halon) with ESMTPS id 0b0ca61c-e203-11e9-969c-12813bfff9fa; Sat, 28 Sep 2019 15:17:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.de; i=@amazon.de; q=dns/txt; s=amazon201209; t=1569683831; x=1601219831; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version; bh=cyGVTeJqWDg2m+WVvSYBoBJJDjh9xXyGK18Rrdrna3o=; b=MEPFdmXEe9XXmb8XdbFmfCMdSaMEYf6KTzZ9FVdBMG9d/gjtzhcEa7nc tf9+VL+4KLajoz92e3PBX/mwK35SIwKW1lei5wGBDxVhxjrLHJgq3XYY3 /SIhD7tfy07dX2bKigecmhUH9H/NhU/MprGpaSMjHvswvPPtUtfRTSuCP U=; X-IronPort-AV: E=Sophos;i="5.64,559,1559520000"; d="scan'208";a="837593963" Received: from sea3-co-svc-lb6-vlan2.sea.amazon.com (HELO email-inbound-relay-1e-27fb8269.us-east-1.amazon.com) ([10.47.22.34]) by smtp-border-fw-out-33001.sea14.amazon.com with ESMTP; 28 Sep 2019 15:14:18 +0000 Received: from EX13MTAUEA001.ant.amazon.com (iad55-ws-svc-p15-lb9-vlan3.iad.amazon.com [10.40.159.166]) by email-inbound-relay-1e-27fb8269.us-east-1.amazon.com (Postfix) with ESMTPS id 48EB0A1CBA; Sat, 28 Sep 2019 15:13:51 +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:43 +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:42 +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:38 +0000 From: Pawel Wieczorkiewicz To: Date: Sat, 28 Sep 2019 15:12:58 +0000 Message-ID: <20190928151305.127380-6-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 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 livepatch 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 livepatching system's agility and provide more flexible long-term livepatch 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 livepatch 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 livepatch 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 v3: * s/hotpatch/livepatch/g * remove extra newline Changed since v1: * added corresponding documentation * added tests docs/misc/livepatch.pandoc | 23 ++++++++ xen/common/livepatch.c | 65 ++++++++++++++++++---- xen/include/xen/livepatch_payload.h | 10 ++++ xen/test/livepatch/Makefile | 10 +++- xen/test/livepatch/xen_action_hooks.c | 100 ++++++++++++++++++++++++++++++++++ 5 files changed, 197 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 5abddd0d27..e6c218ccf7 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 e8ba9317b7..38b1e3519d 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,17 @@ 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 +1251,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: + */