Re: [edk2-devel] [PATCH 1/1] ArmPkg: Add Pcd to disable EFI_MEMORY_ATTRIBUTE_PROTOCOL

["Ard Biesheuvel" <ardb@kernel.org>]

On Tue, 20 Jun 2023 at 19:07, Sean Brogan <spbrogan@outlook.com> wrote:
>
> I don't think a MemoryAttributes2Protocol with a single API would have avoided the errors.
>
> The programming pattern that triggered this would still need multiple calls to any API and in the future where all memory is allocated as NX this possibility would still exist.
>
> A short term effort to minimize the compatibility problem in the ecosystem is documented here Memory Protections: Document compatibility challenges · Issue #18 · tianocore/projects (github.com)  It does not address (and i don't see any reason to try to) a loader that uses the protocol incorrectly.
>
> We have provided virtual reference platforms with these features enabled (both arm and x86) and have been working with the relevant communities for multiple years now.  The UEFI CA for option roms already have compliance requirements (UPDATED: UEFI Signing Requirements - Microsoft Community Hub).  But there are and will continue to be compatibility challenges when enabling a more restrictive execution environment in uefi and the uefi ecosystem.  The more things we make optional the longer this transition period will take.    "Memory Mitigations" were proposed and mostly coded over a decade ago.  The code changes are not that difficult. To change our vast and unwieldy ecosystem is the hard part.   Please help to "stay the course" for this very necessary industry change.   If a production platform has requirements that force such a configuration option that is understandable but it is counter productive in open-source industry standard reference Edk2 code.
>

Fair enough.

But I will note that the only reason we are in this situation in the
first place is because shim has to re-implement the PE loader, cert
handling and all related crypto, and needs the memory attributes
protocol to manipulate the RO/NX permissions.

Now that we are taking these things seriously, wouldn't it be *much*
better to get rid of all this cruft, and specify a method by which a
loader can provide an ephemeral DB that the system firmware will
authenticate against? That way, we can reduce shim to a single
SetVariable() call that creates the ephemeral DB (and perhaps a call
into the TPM code to measure it), which is arguably a lot easier to
audit than the code we have today.