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 A4D76C4708E for ; Thu, 5 Jan 2023 22:56:46 +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=6zluapgEGrFPRxhUidZ9CYlcd+pKAaMIGYMtK2zIKZo=; b=nLpHh/kqlJODZT qdphzdTWEmZeSHvK4qti6Gzpjim250cM0JvaImkPN0Fw1CmA1KqkRisfBQfZTwUwmRlPLP5eLrusC EYsJ85Y/iQwj9gydu7aE1Ws8yfy0ASMMg2ft1rUawi/j2TRg3+uyUqXVYf33kt1QN93jYV27Oxrpo plKDNFcBHxiNIZVPKTYqIg/C7W+sMY1NL8r2jftGFEOxtVF5jt4Q5+RrFNioauMxCMoFRUDGsFm2a YHHvjczcM4Fs/jUD+nJZH1E97zEEsSRXo3TCG8mnzVv6FTgD06ytREL4M233PepV6M92RjxzPbrvo ijNhtUNK8m55Ri+zMf8w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pDZ9B-00FqE5-HO; Thu, 05 Jan 2023 22:55:41 +0000 Received: from desiato.infradead.org ([2001:8b0:10b:1:d65d:64ff:fe57:4e05]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pDVPr-00Dt6v-NJ for linux-arm-kernel@bombadil.infradead.org; Thu, 05 Jan 2023 18:56:39 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=eGwoAoZj2IFde6XBAe3HErVmGZVrADuaXexxrflfQ8o=; b=gUd9nFWp7DdJg7n0x0seoVZPwB i2YJ7U8M3wwhbQ2iamohYnEmZZJrYHm6oAFj1C8/SyKTkzPH1fW/G8HmzBvQU59u3/EyaVRvHElae Ck7MPjTk0kt2F9dF2QekgXQxGPvSeZZkUvEaILGP6rh3jx0XQtQeH4c24ByURTATmQKlEPze07/hg jVzlBVZ8N7ZqkENA7+RhbdQFqAXR/dp+N6woj+gOqtqJWOtDwQ/2MChqZ2rt1f645tIPoCmMvQSxF KPFPd8qGX9dNzHo2CVkuJ5JWLSJELOmsvKH0impyRBYbnu9ZI2Uxhd/0duwnrpSPDesFy9BSrc8Kg j1kKvZow==; Received: from foss.arm.com ([217.140.110.172]) by desiato.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1pDPnN-001K4B-0i for linux-arm-kernel@lists.infradead.org; Thu, 05 Jan 2023 12:56:35 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B566315BF; Thu, 5 Jan 2023 04:57:14 -0800 (PST) Received: from FVFF77S0Q05N (unknown [10.57.45.56]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7BACE3F663; Thu, 5 Jan 2023 04:56:31 -0800 (PST) Date: Thu, 5 Jan 2023 12:56:28 +0000 From: Mark Rutland To: Ard Biesheuvel Cc: Lee Jones , stable@vger.kernel.org, linux-efi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, will@kernel.org, catalin.marinas@arm.com, Sami Tolvanen , Kees Cook Subject: Re: [PATCH 1/2] arm64: efi: Execute runtime services from a dedicated stack Message-ID: References: <20221205201210.463781-1-ardb@kernel.org> <20221205201210.463781-2-ardb@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230105_125633_697444_8A5E9086 X-CRM114-Status: GOOD ( 41.78 ) 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="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Jan 04, 2023 at 05:32:18PM +0100, Ard Biesheuvel wrote: > On Wed, 4 Jan 2023 at 17:30, Mark Rutland wrote: > > > > On Wed, Jan 04, 2023 at 05:15:34PM +0100, Ard Biesheuvel wrote: > > > On Wed, 4 Jan 2023 at 17:13, Mark Rutland wrote: > > > > > > > > On Wed, Jan 04, 2023 at 02:56:19PM +0100, Ard Biesheuvel wrote: > > > > > On Wed, 4 Jan 2023 at 11:40, Lee Jones wrote: > > > > > > > > > > > > On Mon, 05 Dec 2022, Ard Biesheuvel wrote: > > > > > > > > > > > > > With the introduction of PRMT in the ACPI subsystem, the EFI rts > > > > > > > workqueue is no longer the only caller of efi_call_virt_pointer() in the > > > > > > > kernel. This means the EFI runtime services lock is no longer sufficient > > > > > > > to manage concurrent calls into firmware, but also that firmware calls > > > > > > > may occur that are not marshalled via the workqueue mechanism, but > > > > > > > originate directly from the caller context. > > > > > > > > > > > > > > For added robustness, and to ensure that the runtime services have 8 KiB > > > > > > > of stack space available as per the EFI spec, introduce a spinlock > > > > > > > protected EFI runtime stack of 8 KiB, where the spinlock also ensures > > > > > > > serialization between the EFI rts workqueue (which itself serializes EFI > > > > > > > runtime calls) and other callers of efi_call_virt_pointer(). > > > > > > > > > > > > > > While at it, use the stack pivot to avoid reloading the shadow call > > > > > > > stack pointer from the ordinary stack, as doing so could produce a > > > > > > > gadget to defeat it. > > > > > > > > > > > > > > Signed-off-by: Ard Biesheuvel > > > > > > > --- > > > > > > > arch/arm64/include/asm/efi.h | 3 +++ > > > > > > > arch/arm64/kernel/efi-rt-wrapper.S | 13 +++++++++- > > > > > > > arch/arm64/kernel/efi.c | 25 ++++++++++++++++++++ > > > > > > > 3 files changed, 40 insertions(+), 1 deletion(-) > > > > > > > > > > > > Could we have this in Stable please? > > > > > > > > > > > > Upstream commit: ff7a167961d1b ("arm64: efi: Execute runtime services from a dedicated stack") > > > > > > > > > > > > Ard, do we need Patch 2 as well, or can this be applied on its own? > > > > > > > > > > > > > > > > Thanks for the reminder. > > > > > > > > > > Only patch #1 is needed. It should be applied to v5.10 and later. > > > > > > > > Hold on, why did this go into mainline when I had an outstanding comment w.r.t. > > > > the stack unwinder? > > > > > > > > From your last reply to me there I was expecting a respin with that fixed. > > > > > > > > > > Apologies for the confusion. > > > > > > I have a patch for this queued up, but AIUI, that cannot be merged all > > > the way back to v5.10, so these need to remain separate changes in any > > > case. > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=c2530a04a73e6b75ed71ed14d09d7b42d6300013 > > > > Ah, ok, thanks for the pointer! > > > > I'm a little uneasy here, still. > > > > By backporting this we're also backporting the new breakage of the stack > > unwinder, and the minimal change for backports would be to add the lock and not > > the new stack (which was added for additinoal robustness, not to fix the bug > > the lock fixes). > > > > I do appreciate that the additional stack is likely more useful than the > > occasional diagnostic output from the kernel, but it does seem like this has > > traded off one bug for another, and I'm just a little annoyed because I pointed > > that out before the first pull request was made. > > > > I do know that this isn't malicious, and I'm not trying to start a fight, but > > now we have to consider whether we want/need to backport a stack unwinder fix > > to account for this, and we hadn't had that discussion before. > > In that case, let's drop these backports for the time being, and > collaborate on a solution that works for all of us. Thanks! IIUC our options here are: 1) Create a cut-down patch for stable that just adds the new lock but leaves out the new stack. I may be missing a reason why that's insufficient or painful. 2) Backport this *but* also backport the follow-up fixes from your other series: https://lore.kernel.org/r/20230104174433.1259428-1-ardb@kernel.org Above you mentioned something about v5.10, was that just to say that some manual backporting was required, or that there was a structural problem that would require more invasive changes / prerequisites? 3) Something else? My preference would be (1), but if we are encountering issue with stack size on stable kernels, then I'd be happy to help with manual backporting effort for (2), as long as we backported all the relevant bits in one go. Does that make sense, and does that sound reasonable to you? Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel