From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 43EEBC25B10 for ; Mon, 13 May 2024 17:21:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Mud8faWb+EKSUmkyVLZFqkOtUlLjf1WwAtSlAPu8VFM=; b=MaGCY91zW1DcEt WsghRvB7kQAmTCzMf7cNA/crN8RXvEUuu2tdwNNK9ItPgm2R91QrQi8ss2VZtTP2DACOnAauulAC5 KaoHeZaTaEMuUkU2+meHvfw21XF7Ej15psBPsxphzaFIHUAFj3g3gSFGM0w/xtsIFLDntz/Fu4ajK PRCUAPUdRG/qSX//S81va/gpFvIDmEiDbmjwTk7pVu8RjIc5tqFrLNFI4A8i0f6dTAa+MzmwcdAzv u8KuVNkzLh4VAFtV2yQMg16Yp2QVkp8SQ5xb+FkFa0sLUBkOv7ectbPvtYc15iL1TY3lpkq+4/ct6 osv5O+P3qYTe+NmzrdkQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1s6ZMj-0000000Dhp2-0sG5; Mon, 13 May 2024 17:21:33 +0000 Received: from sin.source.kernel.org ([2604:1380:40e1:4800::1]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1s6ZMf-0000000Dhnx-0EOh for linux-arm-kernel@lists.infradead.org; Mon, 13 May 2024 17:21:31 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 2E15CCE0F1C; Mon, 13 May 2024 17:21:27 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B77F7C113CC; Mon, 13 May 2024 17:21:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1715620886; bh=k++p0ZG9E1nTiNJDIztVZOSY2K/lTio1vsTURr954Oc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ZX0cQ39zWPuJUcRR0OHvx7IUl9CqRcakfXqWjz3zW2Bv+aKK/Sr+V5Li7aTqODExu tB+6QTxPLdfjJWoAg2wkmNQW6RnTqTWapbP/8Rxhie8fK/Yo0WHuKI/FG7EiSIVu4T NrpHzv3F3BUJe0zvcVeHaYIJUDQdkwzns5gT3Ii+muhLm26G4JmEdEJ4EdQXJwGbmb 44CnUrUnhxGOyku/afuKyLw6VvezfXpMiKELWfKFSU1G2CRJr02Lt+6GO1768spit0 BbHYqqbmss6JdfY3pjvNgL7wUhnaw9A5gSVzhGT6Bm5TQftgp5TmtDawj+Exm+wHJs mpv5XjxMfe6GQ== Date: Mon, 13 May 2024 18:21:21 +0100 From: Will Deacon To: =?iso-8859-1?Q?Pierre-Cl=E9ment?= Tosi Cc: kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, Marc Zyngier , Oliver Upton , Suzuki K Poulose , Vincent Donnefort Subject: Re: [PATCH v3 09/12] KVM: arm64: VHE: Add test module for hyp kCFI Message-ID: <20240513172120.GA29051@willie-the-truck> References: <20240510112645.3625702-1-ptosi@google.com> <20240510112645.3625702-10-ptosi@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240510112645.3625702-10-ptosi@google.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240513_102129_501022_928BF129 X-CRM114-Status: GOOD ( 40.93 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, May 10, 2024 at 12:26:38PM +0100, Pierre-Cl=E9ment Tosi wrote: > In order to easily periodically (and potentially automatically) validate > that the hypervisor kCFI feature doesn't bitrot, introduce a way to > trigger hypervisor kCFI faults from userspace on test builds of KVM. > = > Add hooks in the hypervisor code to call registered callbacks (intended > to trigger kCFI faults either for the callback call itself of from > within the callback function) when running with guest or host VBAR_EL2. > As the calls are issued from the KVM_RUN ioctl handling path, userspace > gains control over when the actual triggering of the fault happens > without needing to modify the KVM uAPI. > = > Export kernel functions to register these callbacks from modules and > introduce a kernel module intended to contain any testing logic. By > limiting the changes to the core kernel to a strict minimum, this > architectural split allows tests to be updated (within the module) > without the need to redeploy (or recompile) the kernel (hyp) under test. > = > Use the module parameters as the uAPI for configuring the fault > condition being tested (i.e. either at insertion or post-insertion > using /sys/module/.../parameters), which naturally makes it impossible > for userspace to test kCFI without the module (and, inversely, makes > the module only - not KVM - responsible for exposing said uAPI). > = > As kCFI is implemented with a caller-side check of a callee-side value, > make the module support 4 tests based on the location of the caller and > callee (built-in or in-module), for each of the 2 hypervisor contexts > (host & guest), selected by userspace using the 'guest' or 'host' module > parameter. For this purpose, export symbols which the module can use to > configure the callbacks for in-kernel and module-to-built-in kCFI > faulting calls. > = > Define the module-to-kernel API to allow the module to detect that it > was loaded on a kernel built with support for it but which is running > without a hypervisor (-ENXIO) or with one that doesn't use the VHE CPU > feature (-EOPNOTSUPP), which is currently the only mode for which KVM > supports hypervisor kCFI. > = > Allow kernel build configs to set CONFIG_HYP_CFI_TEST to only support > the in-kernel hooks (=3Dy) or also build the test module (=3Dm). Use > intermediate internal Kconfig flags (CONFIG_HYP_SUPPORTS_CFI_TEST and > CONFIG_HYP_CFI_TEST_MODULE) to simplify the Makefiles and #ifdefs. As > the symbols for callback registration are only exported to modules when > CONFIG_HYP_CFI_TEST !=3D n, it is impossible for the test module to be > non-forcefully inserted on a kernel that doesn't support it. > = > Note that this feature must NOT result in any noticeable change > (behavioral or binary size) when HYP_CFI_TEST_MODULE =3D n. > = > CONFIG_HYP_CFI_TEST is intentionally independent of CONFIG_CFI_CLANG, to > avoid arbitrarily limiting the number of flag combinations that can be > tested with the module. > = > Also note that, as VHE aliases VBAR_EL1 to VBAR_EL2 for the host, > testing hypervisor kCFI in VHE and in host context is equivalent to > testing kCFI support of the kernel itself i.e. EL1 in non-VHE and/or in > non-virtualized environments. For this reason, CONFIG_CFI_PERMISSIVE > **will** prevent the test module from triggering a hyp panic (although a > warning still gets printed) in that context. > = > Signed-off-by: Pierre-Cl=E9ment Tosi > --- > arch/arm64/include/asm/kvm_cfi.h | 36 ++++++++ > arch/arm64/kvm/Kconfig | 22 +++++ > arch/arm64/kvm/Makefile | 3 + > arch/arm64/kvm/hyp/include/hyp/cfi.h | 47 ++++++++++ > arch/arm64/kvm/hyp/vhe/Makefile | 1 + > arch/arm64/kvm/hyp/vhe/cfi.c | 37 ++++++++ > arch/arm64/kvm/hyp/vhe/switch.c | 7 ++ > arch/arm64/kvm/hyp_cfi_test.c | 43 +++++++++ > arch/arm64/kvm/hyp_cfi_test_module.c | 133 +++++++++++++++++++++++++++ > 9 files changed, 329 insertions(+) > create mode 100644 arch/arm64/include/asm/kvm_cfi.h > create mode 100644 arch/arm64/kvm/hyp/include/hyp/cfi.h > create mode 100644 arch/arm64/kvm/hyp/vhe/cfi.c > create mode 100644 arch/arm64/kvm/hyp_cfi_test.c > create mode 100644 arch/arm64/kvm/hyp_cfi_test_module.c > = > diff --git a/arch/arm64/include/asm/kvm_cfi.h b/arch/arm64/include/asm/kv= m_cfi.h > new file mode 100644 > index 000000000000..13cc7b19d838 > --- /dev/null > +++ b/arch/arm64/include/asm/kvm_cfi.h > @@ -0,0 +1,36 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (C) 2024 - Google Inc > + * Author: Pierre-Cl=E9ment Tosi > + */ > + > +#ifndef __ARM64_KVM_CFI_H__ > +#define __ARM64_KVM_CFI_H__ > + > +#include > +#include > + > +#ifdef CONFIG_HYP_SUPPORTS_CFI_TEST > + > +int kvm_cfi_test_register_host_ctxt_cb(void (*cb)(void)); > +int kvm_cfi_test_register_guest_ctxt_cb(void (*cb)(void)); Hmm, I tend to think this indirection is a little over the top for a test module that only registers a small handful of handlers: > +static int set_param_mode(const char *val, const struct kernel_param *kp, > + int (*register_cb)(void (*)(void))) > +{ > + unsigned int *mode =3D kp->arg; > + int err; > + > + err =3D param_set_uint(val, kp); > + if (err) > + return err; > + > + switch (*mode) { > + case 0: > + return register_cb(NULL); > + case 1: > + return register_cb(hyp_trigger_builtin_cfi_fault); > + case 2: > + return register_cb((void *)hyp_cfi_builtin2module_test_target); > + case 3: > + return register_cb(trigger_module2builtin_cfi_fault); > + case 4: > + return register_cb(trigger_module2module_cfi_fault); > + default: > + return -EINVAL; > + } > +} Why not just have a hyp selftest that runs through all of this behind a static key? I think it would simplify the code quite a bit, and you could move the registration and indirection logic. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel