From patchwork Tue Nov 5 19:43:16 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Cooper X-Patchwork-Id: 11228469 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 B56371850 for ; Tue, 5 Nov 2019 19:45:01 +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 9C38021A4A for ; Tue, 5 Nov 2019 19:45:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=citrix.com header.i=@citrix.com header.b="PIc2DLE3" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9C38021A4A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=citrix.com 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 1iS4jj-0005g2-6v; Tue, 05 Nov 2019 19:43:31 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iS4jh-0005fr-IQ for xen-devel@lists.xenproject.org; Tue, 05 Nov 2019 19:43:29 +0000 X-Inumbo-ID: 87f42c08-0004-11ea-9631-bc764e2007e4 Received: from esa3.hc3370-68.iphmx.com (unknown [216.71.145.155]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 87f42c08-0004-11ea-9631-bc764e2007e4; Tue, 05 Nov 2019 19:43:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1572983005; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version; bh=hPrZvzQfRdYVwJuZQQi3nJdPY4SK4z9tSv3EXbIdUR4=; b=PIc2DLE3XbvFi07k61uKF7dyMzplioUYjAZxSLr5XUmArW33cbSxRJRB kzrXHlWgDTLDXkQskYe7jKr4gOOBm74JPjXKlWk+Xhq4R2SJqsfat4kLX P0J0g347j88FuGsCEftF8NLCxByA1217EkzX7Que5ZdERfXDkc6VHsW/4 c=; Authentication-Results: esa3.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=andrew.cooper3@citrix.com; spf=Pass smtp.mailfrom=Andrew.Cooper3@citrix.com; spf=None smtp.helo=postmaster@mail.citrix.com Received-SPF: None (esa3.hc3370-68.iphmx.com: no sender authenticity information available from domain of andrew.cooper3@citrix.com) identity=pra; client-ip=162.221.158.21; receiver=esa3.hc3370-68.iphmx.com; envelope-from="Andrew.Cooper3@citrix.com"; x-sender="andrew.cooper3@citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa3.hc3370-68.iphmx.com: domain of Andrew.Cooper3@citrix.com designates 162.221.158.21 as permitted sender) identity=mailfrom; client-ip=162.221.158.21; receiver=esa3.hc3370-68.iphmx.com; envelope-from="Andrew.Cooper3@citrix.com"; x-sender="Andrew.Cooper3@citrix.com"; x-conformance=sidf_compatible; x-record-type="v=spf1"; x-record-text="v=spf1 ip4:209.167.231.154 ip4:178.63.86.133 ip4:195.66.111.40/30 ip4:85.115.9.32/28 ip4:199.102.83.4 ip4:192.28.146.160 ip4:192.28.146.107 ip4:216.52.6.88 ip4:216.52.6.188 ip4:162.221.158.21 ip4:162.221.156.83 ip4:168.245.78.127 ~all" Received-SPF: None (esa3.hc3370-68.iphmx.com: no sender authenticity information available from domain of postmaster@mail.citrix.com) identity=helo; client-ip=162.221.158.21; receiver=esa3.hc3370-68.iphmx.com; envelope-from="Andrew.Cooper3@citrix.com"; x-sender="postmaster@mail.citrix.com"; x-conformance=sidf_compatible IronPort-SDR: n/ntTUMBHdXfDhIsaZ//MzxxQ+rF4cFJL1QaSTcBG5CvludCFNnttdzeRxDBOD2vuL0SRotDCL x5f55ExKUEpm64w6sodE/IoN81GVpOuJxFjyXjAg8PA9TiG2BWdPR4ChTDpJ/MazAo9zYQD+K3 AGDtczbxFxKJIu+ehxssdbX7zVzpY1TbXLsU0KnTnhpb10VLV4rY0jfEbj2jh5Mh6tYGOEuQwC 3HqydbyZalgBrW1kY7sw1bABPQyaUAjIrXIJHbqqgqdLqchUW6u8xt0hZQX/9JQJNbXRNkrcUk JhQ= X-SBRS: 2.7 X-MesageID: 7892190 X-Ironport-Server: esa3.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.68,271,1569297600"; d="scan'208";a="7892190" From: Andrew Cooper To: Xen-devel Date: Tue, 5 Nov 2019 19:43:16 +0000 Message-ID: <20191105194317.16232-2-andrew.cooper3@citrix.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20191105194317.16232-1-andrew.cooper3@citrix.com> References: <20191105194317.16232-1-andrew.cooper3@citrix.com> MIME-Version: 1.0 Subject: [Xen-devel] [PATCH 1/2] xen/livepatch: Add a return value to load hooks X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Juergen Gross , Andrew Cooper , Ross Lagerwall , Konrad Rzeszutek Wilk Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" One use of load hooks is to perform a safety check of the system in its quiesced state. Any non-zero return value from a load hook will abort the apply attempt. Signed-off-by: Andrew Cooper Reviewed-by: Ross Lagerwall --- CC: Konrad Rzeszutek Wilk CC: Ross Lagerwall CC: Juergen Gross For several years, the following patch in the series has been shipped in every XenServer livepatch, implemented as a void load hook which modifies its return address to skip to the end of apply_payload(). This method is distinctly less hacky. --- xen/common/livepatch.c | 30 +++++++++++++++++++----------- xen/include/xen/livepatch_payload.h | 2 +- xen/test/livepatch/xen_hello_world.c | 12 +++++++++--- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index 7caa30c202..962647616a 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -1076,25 +1076,33 @@ static int apply_payload(struct payload *data) * temporarily disable the spin locks IRQ state checks. */ spin_debug_disable(); - for ( i = 0; i < data->n_load_funcs; i++ ) - data->load_funcs[i](); + for ( i = 0; !rc && i < data->n_load_funcs; i++ ) + rc = data->load_funcs[i](); spin_debug_enable(); + if ( rc ) + printk(XENLOG_ERR LIVEPATCH "%s: load_funcs[%u] failed: %d\n", + data->name, i, rc); + ASSERT(!local_irq_is_enabled()); - for ( i = 0; i < data->nfuncs; i++ ) - arch_livepatch_apply(&data->funcs[i]); + if ( !rc ) + for ( i = 0; i < data->nfuncs; i++ ) + arch_livepatch_apply(&data->funcs[i]); arch_livepatch_revive(); - /* - * We need RCU variant (which has barriers) in case we crash here. - * The applied_list is iterated by the trap code. - */ - list_add_tail_rcu(&data->applied_list, &applied_list); - register_virtual_region(&data->region); + if ( !rc ) + { + /* + * We need RCU variant (which has barriers) in case we crash here. + * The applied_list is iterated by the trap code. + */ + list_add_tail_rcu(&data->applied_list, &applied_list); + register_virtual_region(&data->region); + } - return 0; + return rc; } static int revert_payload(struct payload *data) diff --git a/xen/include/xen/livepatch_payload.h b/xen/include/xen/livepatch_payload.h index 4a1a96d054..3788b52ddf 100644 --- a/xen/include/xen/livepatch_payload.h +++ b/xen/include/xen/livepatch_payload.h @@ -9,7 +9,7 @@ * The following definitions are to be used in patches. They are taken * from kpatch. */ -typedef void livepatch_loadcall_t(void); +typedef int livepatch_loadcall_t(void); typedef void livepatch_unloadcall_t(void); /* diff --git a/xen/test/livepatch/xen_hello_world.c b/xen/test/livepatch/xen_hello_world.c index 02f3f85dc0..d720001f07 100644 --- a/xen/test/livepatch/xen_hello_world.c +++ b/xen/test/livepatch/xen_hello_world.c @@ -16,9 +16,11 @@ static const char hello_world_patch_this_fnc[] = "xen_extra_version"; extern const char *xen_hello_world(void); static unsigned int cnt; -static void apply_hook(void) +static int apply_hook(void) { printk(KERN_DEBUG "Hook executing.\n"); + + return 0; } static void revert_hook(void) @@ -26,10 +28,14 @@ static void revert_hook(void) printk(KERN_DEBUG "Hook unloaded.\n"); } -static void hi_func(void) +static int hi_func(void) { printk(KERN_DEBUG "%s: Hi! (called %u times)\n", __func__, ++cnt); + + return 0; }; +/* Safe to cast away the return value for this trivial case. */ +void (void_hi_func)(void) __attribute__((alias("hi_func"))); static void check_fnc(void) { @@ -43,7 +49,7 @@ LIVEPATCH_UNLOAD_HOOK(revert_hook); /* Imbalance here. Two load and three unload. */ LIVEPATCH_LOAD_HOOK(hi_func); -LIVEPATCH_UNLOAD_HOOK(hi_func); +LIVEPATCH_UNLOAD_HOOK(void_hi_func); LIVEPATCH_UNLOAD_HOOK(check_fnc);