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

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

On Tue, 3 Jan 2023 at 23:47, dann frazier <dann.frazier@canonical.com> wrote:
>
> On Tue, Jan 03, 2023 at 08:39:24PM +0100, Alexander Graf wrote:
> > 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.
>
> While it has its own issues, I suppose another way to go about it is
> for distributors to provide multiple AAVMF_CODE images, and perhaps
> invent a "feature" flag for the json descriptor for management tools
> to select an appropriate one.
>

I don't think having different versions of the image makes sense, tbh,
but of course, this is up to the distros.

Compatibility with ancient downstream GRUB builds is not a goal of the
EDK2 upstream, so as long as distros can tweak the build to their
needs, I don't see a reason to revert this change upstream.