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

[Ard Biesheuvel]

On Thu, 5 Jan 2023 at 09:43, Alexander Graf <agraf at csgraf.de> wrote:
>
>
>
> > Am 05.01.2023 um 09:11 schrieb Gerd Hoffmann <kraxel at redhat.com>:
> >
> >   Hi,
> >
> >> To clarify, I mean something like the patch below, but with an additional
> >> callback notification similar to the Emu one in LoadImage(), so that we can
> >> make sure we only enable the quirk when we load a known-bad grub binary.
> >> That way we still force distros to ship fixed versions of their code, but
> >> enable old code to continue running.
> >
> >> +  /* TODO: Only run this as part of a notify callback in ImageLoad() when
> >> we
> >> +           load a grub binary with a known-broken hash */
> >> +  BOOLEAN is_broken_grub = TRUE;
> >> +  if (is_broken_grub) {
> >> +    RealAllocatePages = gBS->AllocatePages;
> >> +    gBS->AllocatePages = AllocatePagesForceLoaderCode;
> >> +  }
> >
> > You left out the hard part, which is the list of hashes.
>
> Yes, I'd crowd source that list. If someone has vested interest to keep their old grub binaries working, they can send an upstream patch to add their hash :). At least we'd have a path forward to make things work that is not "revert NX enablement".
>
> >  And I suspect
> > you underestimate the number of broken grub binaries in the wild ...
>
> What number would you expect? I'd hope that we get to <100 realistically.
>
> I'm happy to hear about alternatives to this approach. I'm very confident that forcing NX on always is going to have the opposite effect of what we want: Everyone who ships AAVMF binaries will disable NX because they eventually get bug reports from users that their shiny update regressed some legit use case.
>
> The only alternative I can think of would be logic similar to the patch I sent without any grub hash check: Exclude AllocatePages for LoaderData from the NX logic. Keep NX for any other non-code memory type as well as LoaderData pool allocations.
>

Another thing we might consider is trapping exec permission violations
and switching the pages in question from rw- to r-x.

Does GRUB generally load/map executable modules at page granularity?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97981): https://edk2.groups.io/g/devel/message/97981
Mute This Topic: https://groups.io/mt/93922691/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-