Re: [PATCH v4 21/26] efi/x86: Explicitly set sections memory attributes

[Ard Biesheuvel]

On Sat, 11 Mar 2023 at 16:09, Evgeniy Baskov <baskov@xxxxxxxxx> wrote:
>
> On 2023-03-10 18:20, Ard Biesheuvel wrote:
> > On Thu, 15 Dec 2022 at 13:42, Evgeniy Baskov <baskov@xxxxxxxxx> wrote:
> >>
> >> Explicitly change sections memory attributes in efi_pe_entry in case
> >> of incorrect EFI implementations and to reduce access rights to
> >> compressed kernel blob. By default it is set executable due to
> >> restriction in maximum number of sections that can fit before zero
> >> page.
> >>
> >> Tested-by: Peter Jones <pjones@xxxxxxxxxx>
> >> Signed-off-by: Evgeniy Baskov <baskov@xxxxxxxxx>
> >
> > I don't think we need this patch. Firmware that cares about W^X will
> > map the PE image with R-X for text/rodata and RW- for data/bss, which
> > is sufficient, and firmware that doesn't is a lost cause anyway.
>
> This patch were here mainly here to make .rodata non-executable and for
> the UEFI handover protocol, for which attributes are usually not getting
> applied.
>
> Since the UEFI handover protocol is deprecated, I'll exclude patches
> from
> v5 and maybe submit it separately modified to apply attributes only when
> booting via this protocol.
>

I think the issue here is that loaders that use the UEFI handover
protocol use their own implementations of LoadImage/StartImage as
well, and some of those tend to do little more than copy the image
into memory and jump to the EFI handover protocol entry point, without
even accounting for the image size in memory or clearing the bss.

To be honest, even though I understand the reason these had to be
implemented, I'm a bit reluctant to cater for the needs of such
loaders, given that these are all downstream distro forks of GRUB
(with shim) with varying levels of adherence to the PE/COFF spec.

I'm happy to revisit this later if others feel this is important, but
for the moment, I'd prefer it if we could focus on making the x86
image work better with compliant loaders, which is what this series is
primarily about.