| 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
| ||
|
Message-ID: <CAMj1kXH00miQoZcPygSz+C5dWN0U2AhjTn+HfRki2smfMwjWEQ@mail.gmail.com>
Date: Fri, 2 May 2025 15:59:41 +0200
From: Ard Biesheuvel <ardb@...nel.org>
To: Vitaly Kuznetsov <vkuznets@...hat.com>
Cc: x86@...nel.org, linux-efi@...r.kernel.org,
Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
Dave Hansen <dave.hansen@...ux.intel.com>, "H. Peter Anvin" <hpa@...or.com>,
Peter Jones <pjones@...hat.com>, Daniel Berrange <berrange@...hat.com>,
Emanuele Giuseppe Esposito <eesposit@...hat.com>, Gerd Hoffmann <kraxel@...hat.com>,
Greg KH <gregkh@...uxfoundation.org>, Luca Boccassi <bluca@...ian.org>,
Peter Zijlstra <peterz@...radead.org>, Matthew Garrett <mjg59@...f.ucam.org>,
James Bottomley <James.Bottomley@...senpartnership.com>,
Eric Snowberg <eric.snowberg@...cle.com>, Paolo Bonzini <pbonzini@...hat.com>,
Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>, Alexandre Ghiti <alex@...ti.fr>, linux-riscv@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] x86/efi: Implement support for embedding SBAT data
for x86
On Fri, 2 May 2025 at 15:46, Vitaly Kuznetsov <vkuznets@...hat.com> wrote:
>
> Ard Biesheuvel <ardb@...nel.org> writes:
>
> > On Fri, 2 May 2025 at 14:10, Vitaly Kuznetsov <vkuznets@...hat.com> wrote:
> >>
> >> Ard Biesheuvel <ardb@...nel.org> writes:
> >>
> >> > On Mon, 28 Apr 2025 at 12:59, Vitaly Kuznetsov <vkuznets@...hat.com> wrote:
> >> >>
> >> >> Ard Biesheuvel <ardb@...nel.org> writes:
> >> >>
> >> >> > On Thu, 24 Apr 2025 at 10:10, Vitaly Kuznetsov <vkuznets@...hat.com> wrote:
> >>
> >> ...
> >>
> >> >> >> $(obj)/vmlinux: $(vmlinux-objs-y) $(vmlinux-libs-y) FORCE
> >> >> >> $(call if_changed,ld)
> >> >> >>
> >> >> >> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
> >> >> >> index 3b2bc61c9408..d0a27905de90 100644
> >> >> >> --- a/arch/x86/boot/compressed/vmlinux.lds.S
> >> >> >> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
> >> >> >> @@ -49,9 +49,22 @@ SECTIONS
> >> >> >> *(.data.*)
> >> >> >>
> >> >> >> /* Add 4 bytes of extra space for the obsolete CRC-32 checksum */
> >> >> >> +#ifndef CONFIG_EFI_SBAT
> >> >> >> . = ALIGN(. + 4, 0x200);
> >> >> >> +#else
> >> >> >> + /* Avoid gap between '.data' and '.sbat' */
> >> >> >> + . = ALIGN(. + 4, 0x1000);
> >> >> >> +#endif
> >> >> >> _edata = . ;
> >> >> >> }
> >> >> >> +#ifdef CONFIG_EFI_SBAT
> >> >> >> + .sbat : ALIGN(0x1000) {
> >> >> >> + _sbat = . ;
> >> >> >> + *(.sbat)
> >> >> >> + _esbat = ALIGN(0x200);
> >> >> >> + . = _esbat;
> >> >> >> + }
> >> >> >> +#endif
> >> >> >> . = ALIGN(L1_CACHE_BYTES);
> >> >> >> .bss : {
> >> >> >> _bss = . ;
> >> >> >
> >> >> > This looks a bit odd - see below
> >> >> >
> >> >> >> diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
> >> >> >> index b5c79f43359b..ab851490ef74 100644
> >> >> >> --- a/arch/x86/boot/header.S
> >> >> >> +++ b/arch/x86/boot/header.S
> >> >> >> @@ -207,6 +207,19 @@ pecompat_fstart:
> >> >> >> IMAGE_SCN_MEM_READ | \
> >> >> >> IMAGE_SCN_MEM_WRITE # Characteristics
> >> >> >>
> >> >> >> +#ifdef CONFIG_EFI_SBAT
> >> >> >> + .ascii ".sbat\0\0\0"
> >> >> >> + .long ZO__esbat - ZO__sbat # VirtualSize
> >> >> >> + .long setup_size + ZO__sbat # VirtualAddress
> >> >> >> + .long ZO__esbat - ZO__sbat # SizeOfRawData
> >> >> >> + .long setup_size + ZO__sbat # PointerToRawData
> >> >> >> +
> >> >> >> + .long 0, 0, 0
> >> >> >> + .long IMAGE_SCN_CNT_INITIALIZED_DATA | \
> >> >> >> + IMAGE_SCN_MEM_READ | \
> >> >> >> + IMAGE_SCN_MEM_DISCARDABLE # Characteristics
> >> >> >> +#endif
> >> >> >> +
> >> >> >
> >> >> > This puts the .sbat section at the very end of the file. However, the
> >> >> > virtual size of .data is 'ZO__end - ZO__data' not 'ZO__edata -
> >> >> > ZO__data', and so the .sbat section will overlap with .bss in the
> >> >> > memory view of the image.
> >> >>
> >> >> Missed that, will fix, thanks! A stupid question though: does this
> >> >> matter in practice for SBAT? I don't think anyone needs SBAT data after
> >> >> kernel starts booting so we can consider it 'discarded'. BSS data can
> >> >> then do whatever it wants.
> >> >>
> >> >
> >> > It violates the PE/COFF spec, and some PE loaders and signing tools
> >> > are very pedantic about the layout.
> >>
> >> Turns out it the problem is slightly harder to address then I initially
> >> thought.
> >
> > Yeah I was afraid this was going to be tricky.
> >
> > ...
> >
> >> The problem is similar for zboot.
> >
> > How so?
> >
>
> zboot-header.S has:
>
> .ascii ".data\0\0\0"
> .long __data_size
> .long _etext - .Ldoshdr
> .long __data_rawsize
> .long _etext - .Ldoshdr
>
> where the difference between '__data_rawsize' and '__data_size' is:
>
> PROVIDE(__data_rawsize = ABSOLUTE(_edata - _etext));
> PROVIDE(__data_size = ABSOLUTE(_end - _etext));
>
> and "_end" is the end of BSS. So if we put '.sbat' right after '.data'
> then '.data' will cover it too (so we will get an overlap). If we put
> if after '.bss' then we're going to get a hole (size of '.bss') upon
> (aarch64 example):
>
> objcopy -O binary arch/arm64/boot/vmlinuz.efi.elf arch/arm64/boot/vmlinuz.efi
>
> as AFAIU it won't be able to squeeze the binary, it only truncates the
> tail throwing away secions without content (in particular, '.sbat').
>
Ah I misread your patch - I thought .sbat was between .text and .data,
which arguably makes more sense.
Powered by blists - more mailing lists