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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 11 Mar 2023 18:39:08 +0100
From:   Ard Biesheuvel <ardb@...nel.org>
To:     Evgeniy Baskov <baskov@...ras.ru>
Cc:     Borislav Petkov <bp@...en8.de>, Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Alexey Khoroshilov <khoroshilov@...ras.ru>,
        Peter Jones <pjones@...hat.com>,
        "Limonciello, Mario" <mario.limonciello@....com>,
        joeyli <jlee@...e.com>, lvc-project@...uxtesting.org,
        x86@...nel.org, linux-efi@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v4 21/26] efi/x86: Explicitly set sections memory attributes

On Sat, 11 Mar 2023 at 16:09, Evgeniy Baskov <baskov@...ras.ru> wrote:
>
> On 2023-03-10 18:20, Ard Biesheuvel wrote:
> > On Thu, 15 Dec 2022 at 13:42, Evgeniy Baskov <baskov@...ras.ru> 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@...hat.com>
> >> Signed-off-by: Evgeniy Baskov <baskov@...ras.ru>
> >
> > 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.