From: Ard Biesheuvel <ardb@kernel.org> To: Arvind Sankar <nivedita@alum.mit.edu> Cc: Borislav Petkov <bp@alien8.de>, Sergey Shatunov <me@prok.pw>, hpa@zytor.com, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, mingo@redhat.com, Thomas Gleixner <tglx@linutronix.de>, x86@kernel.org, linux-efi <linux-efi@vger.kernel.org>, initramfs@vger.kernel.org, Donovan Tremura <neurognostic@protonmail.ch>, Harald Hoyer <harald@hoyer.xyz> Subject: Re: [PATCH 1/2] x86/boot/compressed/64: Remove .bss/.pgtable from bzImage Date: Mon, 6 Apr 2020 15:29:18 +0200 [thread overview] Message-ID: <CAMj1kXG+34-bK1XuxX5VopkRt1SV1ewUAEmif+aQj5cJQ=9vbA@mail.gmail.com> (raw) In-Reply-To: <20200406132215.GA113388@rani.riverdale.lan> On Mon, 6 Apr 2020 at 15:22, Arvind Sankar <nivedita@alum.mit.edu> wrote: > > On Mon, Apr 06, 2020 at 01:20:42PM +0200, Borislav Petkov wrote: > > On Mon, Apr 06, 2020 at 11:11:21AM +0200, Ard Biesheuvel wrote: > > > Yes, it is in the PE/COFF specification. [0] > > > > > > The whole problem is that we are conflating 'loading a PE/COFF image' > > > with 'copying a PE/COFF image into memory', which are not the same > > > thing. It is not just the layout issue, we are running into other > > > problems with things like UEFI secure boot and TPM-based measured > > > boot, where the fact that omitting the standard LoadImage() boot > > > service (which takes care of these things under the hood) means that > > > you now have to do your own checks and measurements. These things are > > > literally all over the place at the moment, shim, GRUB, systemd-boot > > > etc, with no authoritative spec that describes which component should > > > be doing what. > > > > Sounds to me like what LoadImage() does is what the authoritative spec > > should be. Perhaps we should write it down as "Do what LoadImage() > > does... " and then enumerate the requirements. > > > > > Commit ec93fc371f014a6fb483e3556061ecad4b40735c has the background, but ... > > > > Nice, I like the aspect of letting firmware do only a minimum amount of > > work. :) > > > > > ... I'll look into updating the documentation as well. > > > > Thanks! > > > > > Note that this stuff is hot off the press, so there may be some issues > > > lurking (like this one) that we hadn't thought of yet. > > > > Right. > > > > > Actually, it may be sufficient to #define __efistub_global to > > > __section(.data) like we already do for ARM, to ensure that these > > > global flags are always initialized correctly. (I'll wait for Sergey > > > to confirm that the spurious enabling of the PCI DMA protection > > > resulting from this BSS issue is causing the boot regression) > > Yeah I thought of that as the easiest fix, but it might be safer to > explicitly zero-init in efi_main to avoid future problems in case > someone adds another variable in bss and isn't aware of this obscure > requirement. We actually already have sys_table in bss, but that one is > always initialized. There's also other globals that aren't annotated > (but not in bss by virtue of having initializers). What do you think? > *If* we zero init BSS, I'd prefer for it to be done in the EFI handover protocol entrypoint only. But does that fix the issue that BSS lives outside of the memory footprint of the kernel image? > What do you think of the other problem -- that's actually worse to fix, > as it won't just be when kaslr is disabled, the startup_64 code will do > relocation to the end of init_size and clobber the initrd before getting > to the kaslr code, so it will break as soon as the firmware loads the > "unified kernel image" at a 2Mb-aligned address. The only thing I can > think of is to just unconditionally call efi_relocate_kernel if we were > entered via handover_entry? > Yes, that seems to be the most robust approach.
WARNING: multiple messages have this Message-ID (diff)
From: Ard Biesheuvel <ardb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> To: Arvind Sankar <nivedita-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org> Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>, Sergey Shatunov <me-fHUSW+XspFU@public.gmane.org>, hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org, Linux Kernel Mailing List <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>, x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, linux-efi <linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, initramfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Donovan Tremura <neurognostic-g/b1ySJe57IkP3XJZ0H8fw@public.gmane.org>, Harald Hoyer <harald-PnQE6hpDOpmlVyrhU4qvOw@public.gmane.org> Subject: Re: [PATCH 1/2] x86/boot/compressed/64: Remove .bss/.pgtable from bzImage Date: Mon, 6 Apr 2020 15:29:18 +0200 [thread overview] Message-ID: <CAMj1kXG+34-bK1XuxX5VopkRt1SV1ewUAEmif+aQj5cJQ=9vbA@mail.gmail.com> (raw) In-Reply-To: <20200406132215.GA113388-ZaHV2fEG+eCZQHugZx4kin66tl449arB@public.gmane.org> On Mon, 6 Apr 2020 at 15:22, Arvind Sankar <nivedita-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org> wrote: > > On Mon, Apr 06, 2020 at 01:20:42PM +0200, Borislav Petkov wrote: > > On Mon, Apr 06, 2020 at 11:11:21AM +0200, Ard Biesheuvel wrote: > > > Yes, it is in the PE/COFF specification. [0] > > > > > > The whole problem is that we are conflating 'loading a PE/COFF image' > > > with 'copying a PE/COFF image into memory', which are not the same > > > thing. It is not just the layout issue, we are running into other > > > problems with things like UEFI secure boot and TPM-based measured > > > boot, where the fact that omitting the standard LoadImage() boot > > > service (which takes care of these things under the hood) means that > > > you now have to do your own checks and measurements. These things are > > > literally all over the place at the moment, shim, GRUB, systemd-boot > > > etc, with no authoritative spec that describes which component should > > > be doing what. > > > > Sounds to me like what LoadImage() does is what the authoritative spec > > should be. Perhaps we should write it down as "Do what LoadImage() > > does... " and then enumerate the requirements. > > > > > Commit ec93fc371f014a6fb483e3556061ecad4b40735c has the background, but ... > > > > Nice, I like the aspect of letting firmware do only a minimum amount of > > work. :) > > > > > ... I'll look into updating the documentation as well. > > > > Thanks! > > > > > Note that this stuff is hot off the press, so there may be some issues > > > lurking (like this one) that we hadn't thought of yet. > > > > Right. > > > > > Actually, it may be sufficient to #define __efistub_global to > > > __section(.data) like we already do for ARM, to ensure that these > > > global flags are always initialized correctly. (I'll wait for Sergey > > > to confirm that the spurious enabling of the PCI DMA protection > > > resulting from this BSS issue is causing the boot regression) > > Yeah I thought of that as the easiest fix, but it might be safer to > explicitly zero-init in efi_main to avoid future problems in case > someone adds another variable in bss and isn't aware of this obscure > requirement. We actually already have sys_table in bss, but that one is > always initialized. There's also other globals that aren't annotated > (but not in bss by virtue of having initializers). What do you think? > *If* we zero init BSS, I'd prefer for it to be done in the EFI handover protocol entrypoint only. But does that fix the issue that BSS lives outside of the memory footprint of the kernel image? > What do you think of the other problem -- that's actually worse to fix, > as it won't just be when kaslr is disabled, the startup_64 code will do > relocation to the end of init_size and clobber the initrd before getting > to the kaslr code, so it will break as soon as the firmware loads the > "unified kernel image" at a 2Mb-aligned address. The only thing I can > think of is to just unconditionally call efi_relocate_kernel if we were > entered via handover_entry? > Yes, that seems to be the most robust approach.
next prev parent reply other threads:[~2020-04-06 13:29 UTC|newest] Thread overview: 130+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-01-09 15:02 [PATCH 1/2] x86/boot/compressed/64: Remove .bss/.pgtable from bzImage Arvind Sankar 2020-01-09 15:02 ` [PATCH 2/2] x86/boot/compressed: Remove unnecessary sections " Arvind Sankar 2020-02-06 11:44 ` Kees Cook 2020-02-19 16:55 ` [tip: x86/boot] " tip-bot2 for Arvind Sankar 2020-02-22 5:08 ` [PATCH 2/2] " Nathan Chancellor 2020-02-22 6:55 ` Borislav Petkov 2020-02-22 7:02 ` Nathan Chancellor 2020-02-22 7:21 ` Fangrui Song 2020-02-22 7:42 ` Nathan Chancellor 2020-02-22 15:37 ` Arvind Sankar 2020-02-22 16:44 ` Arvind Sankar 2020-02-22 17:18 ` [PATCH] x86/boot/compressed: Fix compressed kernel linking with lld Arvind Sankar 2020-02-22 18:14 ` Nathan Chancellor 2020-02-22 18:58 ` Fangrui Song 2020-02-22 20:17 ` Arvind Sankar 2020-02-22 21:01 ` Fangrui Song 2020-02-22 23:33 ` Nick Desaulniers 2020-02-22 23:57 ` Arvind Sankar 2020-02-23 19:37 ` [PATCH 0/2] Stop generating .eh_frame sections Arvind Sankar 2020-02-24 4:15 ` Nathan Chancellor 2020-02-24 20:49 ` Nick Desaulniers 2020-02-24 21:53 ` Arvind Sankar 2020-02-24 22:01 ` Nick Desaulniers 2020-02-23 19:37 ` [PATCH 1/2] arch/x86: Use -fno-asynchronous-unwind-tables to suppress " Arvind Sankar 2020-02-24 20:33 ` Nick Desaulniers 2020-02-24 21:05 ` Arvind Sankar 2020-02-24 21:12 ` Fangrui Song 2020-02-24 21:17 ` Nick Desaulniers 2020-02-24 21:22 ` Nick Desaulniers 2020-02-23 19:37 ` [PATCH 2/2] arch/x86: Drop unneeded linker script discard of .eh_frame Arvind Sankar 2020-02-24 20:45 ` Nick Desaulniers 2020-02-24 21:33 ` Arvind Sankar 2020-02-24 21:58 ` Nick Desaulniers 2020-02-24 23:21 ` [PATCH v2 0/2] Stop generating .eh_frame sections Arvind Sankar 2020-02-24 23:21 ` [PATCH v2 1/2] arch/x86: Use -fno-asynchronous-unwind-tables to suppress " Arvind Sankar 2020-02-24 23:28 ` Nathan Chancellor 2020-02-24 23:30 ` Nick Desaulniers 2020-02-25 4:10 ` Kees Cook 2020-02-25 16:53 ` [tip: x86/boot] x86/*/Makefile: " tip-bot2 for Arvind Sankar 2020-02-24 23:21 ` [PATCH v2 2/2] arch/x86: Drop unneeded linker script discard of .eh_frame Arvind Sankar 2020-02-24 23:28 ` Nathan Chancellor 2020-02-24 23:33 ` Nick Desaulniers 2020-02-25 4:11 ` Kees Cook 2020-02-25 16:53 ` [tip: x86/boot] x86/vmlinux: " tip-bot2 for Arvind Sankar 2020-02-23 22:00 ` [PATCH] x86/boot/compressed: Fix compressed kernel linking with lld Kees Cook 2020-02-24 6:06 ` Fangrui Song 2020-02-22 17:29 ` [PATCH 2/2] x86/boot/compressed: Remove unnecessary sections from bzImage Borislav Petkov 2020-02-22 17:53 ` Arvind Sankar 2020-02-22 7:42 ` Borislav Petkov 2020-02-22 16:22 ` Arvind Sankar 2020-02-22 23:20 ` Nick Desaulniers 2020-02-24 13:28 ` Michael Matz 2020-02-24 20:51 ` Nick Desaulniers 2020-02-24 21:28 ` Fangrui Song 2020-02-24 21:48 ` Arvind Sankar 2020-02-24 22:17 ` Fangrui Song 2020-02-24 22:43 ` Arvind Sankar 2020-02-24 22:50 ` Fangrui Song 2020-02-24 23:08 ` Arvind Sankar 2020-02-25 5:35 ` --orphan-handling=warn (was Re: [PATCH 2/2] x86/boot/compressed: Remove unnecessary sections) " Kees Cook 2020-02-25 16:42 ` --orphan-handling=warn Arvind Sankar 2020-02-25 18:29 ` --orphan-handling=warn Arvind Sankar 2020-02-25 19:42 ` --orphan-handling=warn Kees Cook 2020-02-25 20:37 ` --orphan-handling=warn Nick Desaulniers 2020-02-25 22:02 ` --orphan-handling=warn Kees Cook 2020-02-26 1:56 ` --orphan-handling=warn Fangrui Song 2020-02-26 5:35 ` --orphan-handling=warn Kees Cook 2020-02-26 19:11 ` --orphan-handling=warn Kristen Carlson Accardi 2020-02-26 19:26 ` --orphan-handling=warn Nick Desaulniers 2020-02-24 11:37 ` [tip: x86/boot] x86/boot/compressed: Remove .eh_frame section from bzImage tip-bot2 for Arvind Sankar 2020-02-24 16:41 ` Arvind Sankar 2020-02-24 17:16 ` Borislav Petkov 2020-02-24 17:28 ` Arvind Sankar 2020-02-05 16:29 ` [PATCH 1/2] x86/boot/compressed/64: Remove .bss/.pgtable " Arvind Sankar 2020-02-18 18:03 ` Arvind Sankar 2020-02-19 12:09 ` Borislav Petkov 2020-02-19 17:57 ` Arvind Sankar 2020-02-19 18:22 ` Borislav Petkov 2020-02-19 19:06 ` Arvind Sankar 2020-02-06 11:18 ` Kees Cook 2020-02-19 16:55 ` [tip: x86/boot] " tip-bot2 for Arvind Sankar 2020-04-05 15:42 ` [PATCH 1/2] " Sergey Shatunov 2020-04-05 23:18 ` Arvind Sankar 2020-04-06 0:00 ` Sergey Shatunov 2020-04-06 3:51 ` Arvind Sankar 2020-04-06 3:51 ` Arvind Sankar 2020-04-06 7:32 ` Ard Biesheuvel 2020-04-06 8:47 ` Borislav Petkov 2020-04-06 9:11 ` Ard Biesheuvel 2020-04-06 9:11 ` Ard Biesheuvel 2020-04-06 11:20 ` Borislav Petkov 2020-04-06 11:20 ` Borislav Petkov 2020-04-06 13:22 ` Arvind Sankar 2020-04-06 13:29 ` Ard Biesheuvel [this message] 2020-04-06 13:29 ` Ard Biesheuvel 2020-04-06 16:01 ` Arvind Sankar 2020-04-06 16:01 ` Arvind Sankar 2020-04-06 16:22 ` Ard Biesheuvel 2020-04-06 16:22 ` Ard Biesheuvel 2020-04-06 16:52 ` Arvind Sankar 2020-04-06 16:52 ` Arvind Sankar 2020-04-06 16:59 ` Ard Biesheuvel 2020-04-06 18:06 ` [PATCH 1/2] efi/x86: Move efi stub globals from .bss to .data Arvind Sankar 2020-04-06 18:06 ` Arvind Sankar 2020-04-06 18:06 ` [PATCH 2/2] efi/x86: Always relocate the kernel for EFI handover entry Arvind Sankar 2020-04-14 8:20 ` [tip: efi/urgent] " tip-bot2 for Arvind Sankar 2020-04-06 18:29 ` [PATCH 1/2] efi/x86: Move efi stub globals from .bss to .data Ard Biesheuvel 2020-04-06 18:29 ` Ard Biesheuvel 2020-04-08 7:43 ` Dave Young 2020-04-08 7:43 ` Dave Young 2020-04-08 7:49 ` Ard Biesheuvel 2020-04-08 7:49 ` Ard Biesheuvel 2020-04-09 14:39 ` Arvind Sankar 2020-04-09 14:47 ` Ard Biesheuvel 2020-04-09 16:35 ` Arvind Sankar 2020-04-09 16:35 ` Arvind Sankar 2020-04-10 14:47 ` Arvind Sankar 2020-04-10 14:47 ` Arvind Sankar 2020-04-10 15:26 ` Ard Biesheuvel 2020-04-14 14:57 ` Daniel Kiper 2020-04-14 14:57 ` Daniel Kiper 2020-04-10 11:26 ` Thomas Meyer 2020-04-10 14:38 ` Arvind Sankar 2020-04-11 8:50 ` Thomas Meyer 2020-04-14 8:20 ` [tip: efi/urgent] " tip-bot2 for Arvind Sankar 2020-04-06 17:21 ` [PATCH 1/2] x86/boot/compressed/64: Remove .bss/.pgtable from bzImage Borislav Petkov 2020-04-06 17:21 ` Borislav Petkov 2020-04-06 8:44 ` Ard Biesheuvel 2020-04-06 12:36 ` Sergey Shatunov 2020-04-06 13:20 ` Ard Biesheuvel
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAMj1kXG+34-bK1XuxX5VopkRt1SV1ewUAEmif+aQj5cJQ=9vbA@mail.gmail.com' \ --to=ardb@kernel.org \ --cc=bp@alien8.de \ --cc=harald@hoyer.xyz \ --cc=hpa@zytor.com \ --cc=initramfs@vger.kernel.org \ --cc=linux-efi@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=me@prok.pw \ --cc=mingo@redhat.com \ --cc=neurognostic@protonmail.ch \ --cc=nivedita@alum.mit.edu \ --cc=tglx@linutronix.de \ --cc=x86@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.