> Am 05.01.2023 um 12:44 schrieb Ard Biesheuvel <a...@kernel.org>:
>
> On Thu, 5 Jan 2023 at 12:19, Gerd Hoffmann <kra...@redhat.com> wrote:
>>
>> Hi,
>>
>>>> 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.
>>
>> That sounds neat, especially as we can print a big'n'fat warning in that
>> case, so the problem gets attention without actually breaking users.
>>
>
> That, and a sleep(5)
I like the direction this is moving :)
>
>> Looking at the efi calls it looks like edk2 doesn't track the owner of
>> an allocation (say by image handle), so I suspect it is not possible to
>> automatically figure who is to blame?
>>
>>> Does GRUB generally load/map executable modules at page granularity?
>>
>> I don't think so, at least the code handles modules not being page
>> aligned. But I think it's not grub modules, that fix was actually
>> picked up meanwhile. But there are downstream patches for image
>> loader code which look suspicious to me ...
>>
>
> OK, so the GRUB PE/COFF loader strikes again :-(
>
> So shim may be broken in the exact same way, and the things shim loads
> may not adhere to page alignment for the sections. Loading the kernel
> itself this way should be fine, though - we always had section
> alignment in the EFI stub kernel.
>
> The downside of this approach is that it can only be done on a
> page-by-page basis, given that the EFI_LOADER_DATA region in question
> will cover both the .text/.rodata and the .data/.bss of the loaded
> image.
Does it have to be r-x instead of rwx? If we add the warning and sleep(5), that
should hopefully give enough incentive already to OSVs to fix their grub
binaries :). And then we don't even need to think about alignment.
If I map a region as LoaderCode, I get rwx as well, no? So we effectively make
LoaderData behave like LoaderCode plus warning plus sleep.
Alex
>
> Could someone check/confirm whether shim builds need to be take into
> account here? Thanks.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98038): https://edk2.groups.io/g/devel/message/98038
Mute This Topic: https://groups.io/mt/93922691/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
