Re: [edk2-devel] [PATCH v3 03/16] ArmVirtPkg: make EFI_LOADER_DATA non-executable

["Alexander Graf" <agraf@csgraf.de>]

Hey Ard,

On 03.01.23 10:59, Ard Biesheuvel wrote:
> On Thu, 29 Dec 2022 at 19:00, dann frazier <dann.frazier@canonical.com> wrote:
>> On Mon, Nov 28, 2022 at 04:46:10PM +0100, Gerd Hoffmann wrote:
>>> On Mon, Sep 26, 2022 at 10:24:58AM +0200, Ard Biesheuvel wrote:
>>>> When the memory protections were implemented and enabled on ArmVirtQemu
>>>> 5+ years ago, we had to work around the fact that GRUB at the time
>>>> expected EFI_LOADER_DATA to be executable, as that is the memory type it
>>>> allocates when loading its modules.
>>>>
>>>> This has been fixed in GRUB in August 2017, so by now, we should be able
>>>> to tighten this, and remove execute permissions from EFI_LOADER_DATA
>>>> allocations.
>>> Data point: https://bugzilla.redhat.com/show_bug.cgi?id=2149020
>>> tl;dr: fedora 37 grub.efi is still broken.
>> This is also the case with existing Ubuntu releases, as well as
>> AlmaLinux 9.1 and RHEL 8.7[*]. While it does appear to be fixed for
>> the upcoming Ubuntu 23.04 (presumably via [**]), I plan to revert this
>> patch in Debian/Ubuntu until it is more ubiquitous. Do you want to do
>> the same upstream? I'm not sure at what point it would make sense to
>> reintroduce it, given we can't force users to upgrade their bootloaders.
>>
> Thanks for the report.
>
> You can override PCDs on the build command line, so I suggest you use
> that for building these images as long as it is needed.
>
> E.g,, append this to the build.sh command line
>
> --pcd PcdDxeNxMemoryProtectionPolicy=0xC000000000007FD1
>
> to undo the effects of this patch.
>
> I do not intend to revert this patch - the trend under EFI is towards
> much stricter memory permissions, also on the MS side, and this is
> especially important under CC scenarios. And if 5+ years is not
> sufficient for out-of-tree GRUB to catch up, what is the point of
> waiting for it?


I think we need to be smarter here. ArmVirtPkg is used by a lot of 
virtualization software - such as QEMU. Typically, users (and 
developers) expect that an update to a newer version will preserve 
compatibility.

Let me contrive an example: In 1 year, QEMU updates to the latest AAVMF. 
It ships that as part of its pc-bios directory. Suddenly, we 
accidentally break old (immutable!) iso images that used to work. So 
someone that updates QEMU to a newer version will have a regression in 
running a Fedora 37 installation. Or RHEL 8.7 installation. Not good :).

Outside of OSVs providing new iso images, there is also not much you can 
do about this. The grub binary here runs way before any updater code 
that could pull a fixed version from the internet.

What alternatives do we have? Assuming grub is the only offender we have 
to worry about, is there a way we can identify that the allocation came 
from an unpatched version? Or have a database of hashes (with all known 
grub binaries that have this bug in existing isos) that would force 
disable NX protection for it if we hit it? Or disable NX when Secure 
Boot is disabled?

I really think we need to be a bit more creative than make NX mandatory 
in all cases when we know the are immutable images out there that won't 
work with it, otherwise we break very legit use cases.


Alex