From patchwork Tue Nov 26 10:07:51 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Wieczorkiewicz, Pawel" X-Patchwork-Id: 11261841 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 D12C51393 for ; Tue, 26 Nov 2019 10:10: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 A2C0F2089D for ; Tue, 26 Nov 2019 10:10: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="fYREDZgj" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A2C0F2089D 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 1iZXmh-0007qK-9S; Tue, 26 Nov 2019 10:09:27 +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 1iZXmg-0007pd-8N for xen-devel@lists.xenproject.org; Tue, 26 Nov 2019 10:09:26 +0000 X-Inumbo-ID: cf000766-1034-11ea-a39d-12813bfff9fa Received: from smtp-fw-9102.amazon.com (unknown [207.171.184.29]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id cf000766-1034-11ea-a39d-12813bfff9fa; Tue, 26 Nov 2019 10:09:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.de; i=@amazon.de; q=dns/txt; s=amazon201209; t=1574762960; x=1606298960; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version; bh=ktd9RstaodglRuWo5ykSPyV6gtXBwFFoQUq/HwVQMKc=; b=fYREDZgjmpxtnhsosBrFywRGq65z+Ac1RI1+qP4FQCqmCPPmBpXEop1U FHU+itaWjtzlb4Wn2Lvmwn+BZYnznOtFeS04XnDYd6axOH8K2siqQPI3Z Rfe5UPAJeOE2XIVQJuMg32z/gcXJst2z8nw2Buyz2tdw5xEWP5ZkxRcAk 0=; IronPort-SDR: LihcvoA32IJHtuR4uNHXd/0TnJMDQ2gvsdMNlyzckSlMpuYbauqVU+j6wSkWOeC/hhfytPB1+G lmMN0r0qXgGg== X-IronPort-AV: E=Sophos;i="5.69,245,1571702400"; d="scan'208";a="9921106" Received: from sea32-co-svc-lb4-vlan3.sea.corp.amazon.com (HELO email-inbound-relay-1a-807d4a99.us-east-1.amazon.com) ([10.47.23.38]) by smtp-border-fw-out-9102.sea19.amazon.com with ESMTP; 26 Nov 2019 10:08:40 +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-807d4a99.us-east-1.amazon.com (Postfix) with ESMTPS id 2F35AA244B; Tue, 26 Nov 2019 10:08:36 +0000 (UTC) Received: from EX13D03EUC003.ant.amazon.com (10.43.164.192) by EX13MTAUEA001.ant.amazon.com (10.43.61.243) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Tue, 26 Nov 2019 10:08:21 +0000 Received: from EX13MTAUEB001.ant.amazon.com (10.43.60.96) by EX13D03EUC003.ant.amazon.com (10.43.164.192) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Tue, 26 Nov 2019 10:08:20 +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; Tue, 26 Nov 2019 10:08:18 +0000 From: Pawel Wieczorkiewicz To: Date: Tue, 26 Nov 2019 10:07:51 +0000 Message-ID: <20191126100801.124844-3-wipawel@amazon.de> X-Mailer: git-send-email 2.16.5 In-Reply-To: <20191126100801.124844-1-wipawel@amazon.de> References: <20191126100801.124844-1-wipawel@amazon.de> MIME-Version: 1.0 Precedence: Bulk Subject: [Xen-devel] [PATCH v6 02/12] livepatch: Allow to override inter-modules buildid dependency 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: Stefano Stabellini , Julien Grall , Wei Liu , Konrad Rzeszutek Wilk , George Dunlap , Andrew Cooper , Ross Lagerwall , Ian Jackson , mpohlack@amazon.com, Pawel Wieczorkiewicz , Jan Beulich Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" By default Livepatch enforces the following buildid-based dependency chain between livepatch modules: 1) first module depends on given hypervisor buildid 2) every consecutive module depends on previous module's buildid This way proper livepatch stack order is maintained and enforced. While it is important for production livepatches it limits agility and blocks usage of testing or debug livepatches. These kinds of livepatch modules are typically expected to be loaded at any time irrespective of current state of the modules stack. To enable testing and debug livepatches allow user dynamically ignore the inter-modules dependency. In this case only hypervisor buildid match is verified and enforced. To allow userland pass additional paremeters for livepatch actions add support for action flags. Each of the apply, revert, unload and revert action gets additional 32-bit parameter 'flags' where extra flags can be applied in a mask form. Initially only one flag '--nodeps' is added for the apply action. This flag modifies the default buildid dependency check as described above. The global sysctl interface input flag parameter is defined with a single corresponding flag macro: LIVEPATCH_ACTION_APPLY_NODEPS (1 << 0) The userland xen-livepatch tool is modified to support the '--nodeps' flag for apply and load commands. A general mechanism for specifying more flags in the future for apply and other action is however added. Signed-off-by: Pawel Wieczorkiewicz Reviewed-by: Andra-Irina Paraschiv Reviewed-by: Eslam Elnikety Reviewed-by: Petre Eftime Reviewed-by: Leonard Foerster Reviewed-by: Martin Pohlack Reviewed-by: Norbert Manthey Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: Ross Lagerwall --- Changed since v5: * added missing action pad field zeroing Changed since v4: * changed flags field type from uint64_t to uint32_t * added 'pad' field after the changed flags field Changed since v3: * simplified loop in xen-livepatch.c --- docs/misc/livepatch.pandoc | 8 +++ tools/libxc/include/xenctrl.h | 9 ++-- tools/libxc/xc_misc.c | 21 ++++---- tools/misc/xen-livepatch.c | 121 +++++++++++++++++++++++++++++++++++------- xen/common/livepatch.c | 17 ++++-- xen/include/public/sysctl.h | 12 ++++- 6 files changed, 152 insertions(+), 36 deletions(-) diff --git a/docs/misc/livepatch.pandoc b/docs/misc/livepatch.pandoc index fd1f5d0126..cd859bb811 100644 --- a/docs/misc/livepatch.pandoc +++ b/docs/misc/livepatch.pandoc @@ -659,6 +659,10 @@ The caller provides: * `time` The upper bound of time (ns) the cmd should take. Zero means to use the hypervisor default. If within the time the operation does not succeed the operation would go in error state. + * `flags` provides additional parameters for an action: + * *LIVEPATCH_ACTION_APPLY_NODEPS* (1) Apply action ignores inter-module + buildid dependency. Checks only if module is built for given hypervisor by + comparing buildid. * `pad` - *MUST* be zero. The return value will be zero unless the provided fields are incorrect. @@ -676,6 +680,10 @@ The structure is as follow: /* hypervisor default. */ /* Or upper bound of time (ns) */ /* for operation to take. */ + uint32_t flags; /* IN: action flags. */ + /* Provide additional parameters */ + /* for an action. */ + uint32_t pad; /* IN: Always zero. */ }; diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index f4431687b3..b06738c471 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -2605,11 +2605,12 @@ int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start, * to complete them. The `timeout` offers an option to expire the * operation if it could not be completed within the specified time * (in ns). Value of 0 means let hypervisor decide the best timeout. + * The `flags` allows to pass extra parameters to the actions. */ -int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout); -int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout); -int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout); -int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout); +int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout, uint32_t flags); +int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout, uint32_t flags); +int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout, uint32_t flags); +int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint32_t flags); /* * Ensure cache coherency after memory modifications. A call to this function diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c index 8e60b6e9f0..336580135e 100644 --- a/tools/libxc/xc_misc.c +++ b/tools/libxc/xc_misc.c @@ -854,7 +854,8 @@ int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start, static int _xc_livepatch_action(xc_interface *xch, char *name, unsigned int action, - uint32_t timeout) + uint32_t timeout, + uint32_t flags) { int rc; DECLARE_SYSCTL; @@ -880,6 +881,8 @@ static int _xc_livepatch_action(xc_interface *xch, sysctl.u.livepatch.pad = 0; sysctl.u.livepatch.u.action.cmd = action; sysctl.u.livepatch.u.action.timeout = timeout; + sysctl.u.livepatch.u.action.flags = flags; + sysctl.u.livepatch.u.action.pad = 0; sysctl.u.livepatch.u.action.name = def_name; set_xen_guest_handle(sysctl.u.livepatch.u.action.name.name, name); @@ -891,24 +894,24 @@ static int _xc_livepatch_action(xc_interface *xch, return rc; } -int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout) +int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout, uint32_t flags) { - return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_APPLY, timeout); + return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_APPLY, timeout, flags); } -int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout) +int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout, uint32_t flags) { - return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REVERT, timeout); + return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REVERT, timeout, flags); } -int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout) +int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout, uint32_t flags) { - return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_UNLOAD, timeout); + return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_UNLOAD, timeout, flags); } -int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout) +int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint32_t flags) { - return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REPLACE, timeout); + return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REPLACE, timeout, flags); } /* diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c index 3233472157..b469b253ad 100644 --- a/tools/misc/xen-livepatch.c +++ b/tools/misc/xen-livepatch.c @@ -23,18 +23,23 @@ void show_help(void) { fprintf(stderr, "xen-livepatch: live patching tool\n" - "Usage: xen-livepatch [args]\n" + "Usage: xen-livepatch [args] [command-flags]\n" " An unique name of payload. Up to %d characters.\n" "Commands:\n" " help display this help\n" " upload upload file with name\n" " list list payloads uploaded.\n" - " apply apply patch.\n" + " apply [flags] apply patch.\n" + " Supported flags:\n" + " --nodeps Disable inter-module buildid dependency check.\n" + " Check only against hypervisor buildid.\n" " revert revert name patch.\n" " replace apply patch and revert all others.\n" " unload unload name patch.\n" - " load upload and apply .\n" - " name is the name\n", + " load [flags] upload and apply with name as the name\n" + " Supported flags:\n" + " --nodeps Disable inter-module buildid dependency check.\n" + " Check only against hypervisor buildid.\n", XEN_LIVEPATCH_NAME_SIZE); } @@ -225,12 +230,13 @@ static int upload_func(int argc, char *argv[]) return rc; } -/* These MUST match to the 'action_options[]' array slots. */ +/* These MUST match to the 'action_options[]' and 'flag_options[]' array slots. */ enum { ACTION_APPLY = 0, ACTION_REVERT = 1, ACTION_UNLOAD = 2, ACTION_REPLACE = 3, + ACTION_NUM }; struct { @@ -238,7 +244,7 @@ struct { int expected; /* The state to be in after the function. */ const char *name; const char *verb; - int (*function)(xc_interface *xch, char *name, uint32_t timeout); + int (*function)(xc_interface *xch, char *name, uint32_t timeout, uint32_t flags); } action_options[] = { { .allow = LIVEPATCH_STATE_CHECKED, .expected = LIVEPATCH_STATE_APPLIED, @@ -266,6 +272,66 @@ struct { }, }; +/* + * This structure defines supported flag options for actions. + * It defines entries for each action and supports up to 32 + * flags per action. + */ +struct { + const char *name; + const uint32_t flag; +} flag_options[ACTION_NUM][8 * sizeof(uint32_t)] = { + { /* ACTION_APPLY */ + { .name = "--nodeps", + .flag = LIVEPATCH_ACTION_APPLY_NODEPS, + }, + }, + { /* ACTION_REVERT */ + }, + { /* ACTION_UNLOAD */ + }, + { /* ACTION_REPLACE */ + } +}; + +/* + * Parse user provided action flags. + * This function expects to only receive an array of input parameters being flags. + * Expected action is specified via idx paramater (index of flag_options[]). + */ +static int get_flags(int argc, char *argv[], unsigned int idx, uint32_t *flags) +{ + int i, j; + + if ( !flags || idx >= ARRAY_SIZE(flag_options) ) + return -1; + + *flags = 0; + for ( i = 0; i < argc; i++ ) + { + for ( j = 0; j < ARRAY_SIZE(flag_options[idx]); j++ ) + { + if ( !flag_options[idx][j].name ) + goto error; + + if ( !strcmp(flag_options[idx][j].name, argv[i]) ) + { + *flags |= flag_options[idx][j].flag; + break; + } + } + + if ( j == ARRAY_SIZE(flag_options[idx]) ) + goto error; + } + + return 0; +error: + fprintf(stderr, "Unsupported flag: %s.\n", argv[i]); + errno = EINVAL; + return errno; +} + /* The hypervisor timeout for the live patching operation is 30 msec, * but it could take some time for the operation to start, so wait twice * that period. */ @@ -291,8 +357,9 @@ int action_func(int argc, char *argv[], unsigned int idx) char name[XEN_LIVEPATCH_NAME_SIZE]; int rc; xen_livepatch_status_t status; + uint32_t flags; - if ( argc != 1 ) + if ( argc < 1 ) { show_help(); return -1; @@ -301,7 +368,10 @@ int action_func(int argc, char *argv[], unsigned int idx) if ( idx >= ARRAY_SIZE(action_options) ) return -1; - if ( get_name(argc, argv, name) ) + if ( get_name(argc--, argv++, name) ) + return EINVAL; + + if ( get_flags(argc, argv, idx, &flags) ) return EINVAL; /* Check initial status. */ @@ -332,7 +402,7 @@ int action_func(int argc, char *argv[], unsigned int idx) if ( action_options[idx].allow & status.state ) { printf("%s %s... ", action_options[idx].verb, name); - rc = action_options[idx].function(xch, name, HYPERVISOR_TIMEOUT_NS); + rc = action_options[idx].function(xch, name, HYPERVISOR_TIMEOUT_NS, flags); if ( rc ) { int saved_errno = errno; @@ -394,17 +464,23 @@ int action_func(int argc, char *argv[], unsigned int idx) static int load_func(int argc, char *argv[]) { - int rc; - char *new_argv[2]; - char *path, *name, *lastdot; + int i, rc = ENOMEM; + char *upload_argv[2]; + char **apply_argv, *path, *name, *lastdot; - if ( argc != 1 ) + if ( argc < 1 ) { show_help(); return -1; } + + /* apply action has [flags] input requirement, which must be constructed */ + apply_argv = (char **) malloc(argc * sizeof(*apply_argv)); + if ( !apply_argv ) + return rc; + /* */ - new_argv[1] = argv[0]; + upload_argv[1] = argv[0]; /* Synthesize the */ path = strdup(argv[0]); @@ -413,16 +489,23 @@ static int load_func(int argc, char *argv[]) lastdot = strrchr(name, '.'); if ( lastdot != NULL ) *lastdot = '\0'; - new_argv[0] = name; + upload_argv[0] = name; + apply_argv[0] = name; - rc = upload_func(2 /* */, new_argv); + /* Fill in all user provided flags */ + for ( i = 1; i < argc; i++ ) + apply_argv[i] = argv[i]; + + rc = upload_func(2 /* */, upload_argv); if ( rc ) - return rc; + goto error; - rc = action_func(1 /* only */, new_argv, ACTION_APPLY); + rc = action_func(argc, apply_argv, ACTION_APPLY); if ( rc ) - action_func(1, new_argv, ACTION_UNLOAD); + action_func(1 /* only */, upload_argv, ACTION_UNLOAD); +error: + free(apply_argv); free(path); return rc; } diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index a0e0d3092b..3951dccaf2 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -1510,6 +1510,9 @@ static int livepatch_action(struct xen_sysctl_livepatch_action *action) char n[XEN_LIVEPATCH_NAME_SIZE]; int rc; + if ( action->pad ) + return -EINVAL; + rc = get_name(&action->name, n); if ( rc ) return rc; @@ -1583,9 +1586,17 @@ static int livepatch_action(struct xen_sysctl_livepatch_action *action) break; } - rc = build_id_dep(data, !!list_empty(&applied_list)); - if ( rc ) - break; + /* + * Check if action is issued with nodeps flags to ignore module + * stack dependencies. + */ + if ( !(action->flags & LIVEPATCH_ACTION_APPLY_NODEPS) ) + { + rc = build_id_dep(data, !!list_empty(&applied_list)); + if ( rc ) + break; + } + data->rc = -EAGAIN; rc = schedule_work(data, action->cmd, action->timeout); } diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h index 19457a4e30..7a0884b70b 100644 --- a/xen/include/public/sysctl.h +++ b/xen/include/public/sysctl.h @@ -35,7 +35,7 @@ #include "domctl.h" #include "physdev.h" -#define XEN_SYSCTL_INTERFACE_VERSION 0x00000012 +#define XEN_SYSCTL_INTERFACE_VERSION 0x00000013 /* * Read console content from Xen buffer ring. @@ -970,6 +970,16 @@ struct xen_sysctl_livepatch_action { /* hypervisor default. */ /* Or upper bound of time (ns) */ /* for operation to take. */ + +/* + * Override default inter-module buildid dependency chain enforcement. + * Check only if module is built for given hypervisor by comparing buildid. + */ +#define LIVEPATCH_ACTION_APPLY_NODEPS (1 << 0) + uint32_t flags; /* IN: action flags. */ + /* Provide additional parameters */ + /* for an action. */ + uint32_t pad; /* IN: Always zero. */ }; struct xen_sysctl_livepatch_op {