From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.2615.1685051005658038333 for ; Thu, 25 May 2023 14:43:25 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=RrYE2uqF; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 113D964B7A for ; Thu, 25 May 2023 21:43:25 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id EBDC4C433A7 for ; Thu, 25 May 2023 21:43:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1685051004; bh=81Zna8IupdR2FNymTtfO7EHn0xlYiL9rWnrU+ZtoGHI=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=RrYE2uqFsLAk6yYq7PKaW1ywnXo+GxKqoXG32K7zPJyzVSHyp9aWmvklafTky/vnf 29bZUt8OdbxoXij9nFc8KUgThK5+Ewupe/fF+/Nr1oBKINtvqroaMn8BYM5cBHxdQJ sY/nO+tlBrEbUz7rx30DbD8kfcbHjTEA9HhJgSjWP1r0VJ9mfILy12eRsE06cu4i7i YSO8jPzGRU6Ma6jI0locMwsrEWo/XV/9o4q2W+QR5yZy/Y1k6qUNr2zmbRWym66t58 8U3HurQe+TwFYh18IApWJbTjA7d4Sks3yBuey3V9amv+6ZY1yQMGlNO8noksY8qghG PpRLmJCELAD7w== Received: by mail-lf1-f45.google.com with SMTP id 2adb3069b0e04-4f4b2bc1565so2983869e87.2 for ; Thu, 25 May 2023 14:43:23 -0700 (PDT) X-Gm-Message-State: AC+VfDzKJSuZpeQjYK4XviBd+qJ4POXAQAQ4numUWYf5o1ArwtTMK1XF 64vkJ47MTwodmtgZbl4vgM+oyjWLutRtCsskMhA= X-Google-Smtp-Source: ACHHUZ5C2Kbmewb2yudKbEzsdnFVlFYRwqu7/7PKCiRopfbH57i5Yo5fONaIovWihBHBTeV6WbHY0jefpJ7teF1JJrU= X-Received: by 2002:ac2:5df2:0:b0:4f4:cb4c:36e8 with SMTP id z18-20020ac25df2000000b004f4cb4c36e8mr2254418lfq.48.1685051001925; Thu, 25 May 2023 14:43:21 -0700 (PDT) MIME-Version: 1.0 References: <20230525143041.1172989-1-ardb@kernel.org> In-Reply-To: From: "Ard Biesheuvel" Date: Thu, 25 May 2023 23:43:10 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [RFC PATCH 00/10] Add PPI to manage PEI phase memory attributes To: Oliver Smith-Denny Cc: devel@edk2.groups.io, Ray Ni , Jiewen Yao , Gerd Hoffmann , Taylor Beebe , Oliver Smith-Denny , Dandan Bi , Liming Gao , "Kinney, Michael D" , Leif Lindholm , Sunil V L , Andrei Warkentin Content-Type: text/plain; charset="UTF-8" On Thu, 25 May 2023 at 19:21, Oliver Smith-Denny wrote: > > On 5/25/2023 7:30 AM, Ard Biesheuvel wrote: > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4468 > > > > > > > > This is a proof-of-concept RFC that implements a PEI phase PPI to manage > > > > memory permission attributes, and wires it up to the PEI image loader so > > > > that shadowed PEIMs as well as the DXE core are remapped with the > > > > appropriate, restricted memory permission attributes before execution. > > > > > > > > This means that neither shadowed PEIMs nor the DXE core will ever > > > > execute with writable code regions. It also removes the need on the part > > > > of PEI for memory to be mapped with both writable and executable > > > > permissions by default out of reset. Similar work still needs to be done > > > > to address the early DXE phase (before the CPU arch protocol becomes > > > > available), but once that is out of the way as well, platforms should be > > > > able to map all memory non-executable from the beginning. > > > > > > > > This by itself is a major improvement in terms of robustness. It is also > > > > a prerequisite for enabling the WXN MMU control on AArch64, which makes > > > > all writable memory mappings non-executable regardless of the non-exec > > > > page table attribute. > > > > > > > > Patches #1 to #4 are prepatory work. > > > > Patch #5 proposes the memory attribute PPI protocol interface. > > > > Patch #6 implements it for ARM and AARCH64. > > > > Patch #7 wires it up into the PEI image loader. > > > > Patches #8 to #10 update the DxeIpl to use this PPI on ARM/AARCH64 for > > > > mapping the stack NX. > > > > instead of an explicit reference to ArmMmuLib. Other architectures > > > > (except IA32/X64) will seamlessly inherit this once they implement the > > > > PPI as well. > > > > Hi Ard, > Hi, Thanks for taking a look. > Thanks for the RFC. This same issue exists for X64, I saw your mailing > list question about IA32 PEI, do you have plans for extending this to > X64 PEI (I'm not sure its state of readiness)? > Yes, but I need to wrap my head around that code first (I'm not as familiar with low-level X64 as I am with ARM). I though I'd send this RFC out in the meantime to get some alignment on the general shape of things. > If so, do you think it is feasible to merge the X64 DxeLoadFunc with > the generic one you have created? > Hopefully. There are some pieces there that are highly X64 specific, though, but at least the permissions management code could be shared, in which case it can be moved out of the arch-specific source file and into the generic one (this is one of the reasons I merged that code between ARM, AARCH64, RISC-V and LoongArch64). Pure IA32 will simply not implement the PPI, and the long mode switch stuff can remain IA32 specific afaict (although we'll be retaining that only for diagnostic purposes) > Overall, this seems a reasonable approach to me towards making DXE more > protected with the additional benefit of protecting shadowed PEIMs. > > I did wonder if the same discussion we are having on the DXE side about > moving these MMU manipulations to the core instead of the CPU driver > make sense in PEI, too, but with the greatly reduced complexity of the > environment (no GCD, etc.), I don't think it makes sense now and that > focusing on the DXE reorganization and expansion is going to deliver > a much greater impact. > IMHO the modularity has its advantages in some cases, and this particular use case does not force us to deviate from that principle, so I'd rather keep that as a separate discussion. On ARM (and now on X64 for TDX) we already have PEI-less platforms where SEC dispatches the DXE core directly, so I'm not convinced we really need something in between. -- Ard.