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 B172FC27C54 for ; Thu, 6 Jun 2024 16:23:02 +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=x4G5jYvn7bHyihbfpDS43QUyAC7LBuIcYMTMkaKVxr4=; b=dr51sL8CVvo2sM IYLxziCEZBi80EPkodRF/cWGqk0wF6HKBEJWR7hdOu5IBuZpHvigtKXsrJEu3iAtdc1CusoTPbmb7 Y0oCEUhI2/tydAoVdNWu+RopBFxTYwfrTw8hFh6qUgG9H5n/HIWj2nl4POw0ZsujgftnNj2dgItNB cCrW9Kfyai4riDv+LOwiOyuVLoz5PFhxmLzKUAXZRJooRnSwFwQxgwH7ZCUjp7fxlz0yGfrCTnSlV 5dEHcE3eAAZn+iEa9VI79NDIbCldCsyBCzcSUBLrpmQk7QD3nhH3OPVwVJUtYcgHo7JSd8YZ9QKwK TlThrcazLidmaf90h8jw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sFFt2-0000000AUvA-1l0b; Thu, 06 Jun 2024 16:22:48 +0000 Received: from sin.source.kernel.org ([145.40.73.55]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sFFsz-0000000AUtu-1Go2 for linux-arm-kernel@lists.infradead.org; Thu, 06 Jun 2024 16:22:46 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 0DDF5CE1BB4; Thu, 6 Jun 2024 16:22:36 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B9EF7C2BD10; Thu, 6 Jun 2024 16:22:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1717690955; bh=Q+vrgTBGO4uoIOOe7S/ANwtcnFISMpO8zG4tv8qihJs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=RZ0Nhn5wVlF93tjBwFq8HsyrTnvto9/1TGU4wvK3s1CMwRqNYAVPuPjvFn6e02gYn PlPJDfj+AIdI4RtXo6A3n+4ROqzf8faGiM6qdPbwrfH7KnDlARvp1KhAEsJ/RYi9DS np6kEpAn0ky2D44t8U+9G/wVQYvAXxsPC/JKzo4VrYI8dSskEhqM0z9dcBporrJj3m A3u2Z65TWapU00op5WOU8VDcwy/Kz+nkauRj4tfXf1wTn0aWOlhPo7Tk3FwjItkRAo hV2xSOQ4FrTDXZby9lFrrX57r1XWOGcIw3Qh4MqWBI7gMqVcZcbl2PxjIe6kGU3GSE gP0ueFkuunGtQ== Date: Thu, 6 Jun 2024 17:22:26 +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 v4 11/13] KVM: arm64: Improve CONFIG_CFI_CLANG error message Message-ID: <20240606162226.GA23425@willie-the-truck> References: <20240529121251.1993135-1-ptosi@google.com> <20240529121251.1993135-12-ptosi@google.com> <20240603144808.GL19151@willie-the-truck> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: 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-20240606_092245_722542_18A2BFFB X-CRM114-Status: GOOD ( 35.31 ) 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 Tue, Jun 04, 2024 at 05:05:59PM +0100, Pierre-Cl=E9ment Tosi wrote: > On Mon, Jun 03, 2024 at 03:48:08PM +0100, Will Deacon wrote: > > On Wed, May 29, 2024 at 01:12:17PM +0100, Pierre-Cl=E9ment Tosi wrote: > > > For kCFI, the compiler encodes in the immediate of the BRK (which the > > > CPU places in ESR_ELx) the indices of the two registers it used to ho= ld > > > (resp.) the function pointer and expected type. Therefore, the kCFI > > > handler must be able to parse the contents of the register file at the > > > point where the exception was triggered. > > > = > > > To achieve this, introduce a new hypervisor panic path that first sto= res > > > the CPU context in the per-CPU kvm_hyp_ctxt before calling (directly = or > > > indirectly) hyp_panic() and execute it from all EL2 synchronous > > > exception handlers i.e. > > > = > > > - call it directly in host_el2_sync_vect (__kvm_hyp_host_vector, EL2t= &h) > > > - call it directly in el2t_sync_invalid (__kvm_hyp_vector, EL2t) > > > - set ELR_EL2 to it in el2_sync (__kvm_hyp_vector, EL2h), which ERETs > > > = > > > Teach hyp_panic() to decode the kCFI ESR and extract the target and t= ype > > > from the saved CPU context. In VHE, use that information to panic() w= ith > > > a specialized error message. In nVHE, only report it if the host (EL1) > > > has access to the saved CPU context i.e. iff CONFIG_NVHE_EL2_DEBUG=3D= y, > > > which aligns with the behavior of CONFIG_PROTECTED_NVHE_STACKTRACE. > > > = > > > Signed-off-by: Pierre-Cl=E9ment Tosi > > > --- > > > arch/arm64/kvm/handle_exit.c | 30 +++++++++++++++++++++++= -- > > > arch/arm64/kvm/hyp/entry.S | 24 +++++++++++++++++++- > > > arch/arm64/kvm/hyp/hyp-entry.S | 2 +- > > > arch/arm64/kvm/hyp/include/hyp/switch.h | 4 ++-- > > > arch/arm64/kvm/hyp/nvhe/host.S | 2 +- > > > arch/arm64/kvm/hyp/vhe/switch.c | 26 +++++++++++++++++++-- > > > 6 files changed, 79 insertions(+), 9 deletions(-) > > = > > This quite a lot of work just to print out some opaque type numbers > > when CONFIG_NVHE_EL2_DEBUG=3Dy. Is it really worth it? How would I use > > this information to debug an otherwise undebuggable kcfi failure at EL2? > = > The type ID alone might not be worth it but what about the target? > = > In my experience, non-malicious kCFI failures are often caused by an issu= e with > the callee, not the caller. Without this patch, only the source of the ex= ception > is reported but, with it, the panic handler also prints the kCFI target (= i.e. > value of the function pointer) as a symbol. I think it's less of an issue for EL2, as we don't have tonnes of indirections, but I agree that the target is nice to have. > With the infrastructure for the target in place, it isn't much more work = to also > report the type ID. Although it is rarely informative (as you noted), the= re are > some situations where it can still be useful e.g. if reported as zero and= /or has > been corrupted and does not match its value from the ELF. So looking at the implementation, I'm not a huge fan of saving off all the GPRs and then relying on the stage-2 being disabled so that the host can fish out the registers it cares about. I think I'd prefer to provide the target as an additional argument to nvhe_hyp_panic_handler(), meaning that we could even print the VA when CONFIG_NVHE_EL2_DEBUG is disabled. But for now, I suggest we drop this patch along with the testing patches because I think the rest of the series is nearly there and it's a useful change on its own. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel