Re: [PATCH v2 0/7] x86/efi,boot: GDT handling cleanup/fixes

From: Ard Biesheuvel
Date: Sun Feb 02 2020 - 13:02:09 EST


On Sun, 2 Feb 2020 at 18:13, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote:
>
> This series fixes a potential bug in EFI mixed-mode and leaves GDT
> handling to startup_{32,64} instead of efi_main.
>
> The first patch removes KEEP_SEGMENTS support in loadflags, this is
> unused now (details in patch 1 commit msg), to slightly simplify
> subsequent changes.
>
> The second patch fixes a potential bug in EFI mixed-mode, where we are
> currently relying on the firmware GDT having a particular layout: a
> CODE32 segment as descriptor 2 and a DATA segment as descriptor 3.
>
> The third patch adds some safety during kernel decompression by updating
> the GDTR to point to the copied GDT, rather than the old one which may
> have been overwritten.
>
> The fourth patch adds cld/cli to startup_64, and the fifth patch removes
> all the GDT setup from efi_main and adds it to the 32-bit kernel's
> startup_32. The 64-bit kernel already does GDT setup. This should be
> safer as this code can keep track of where the .data section is moving
> and ensure that GDTR is pointing to a clean copy of the GDT.
>
> The last two patches are to fix an off-by-one in the GDT limit and do a
> micro-optimization to the GDT loading instructions.
>

Thanks Arvind.

This looks good to me,

Acked-by: Ard Biesheuvel <ardb@xxxxxxxxxx>

but I'm a bit out of my depth here when it comes to x86'ology so it's
really up to the x86 maintainers.


> Changes from v1:
> - added removal of KEEP_SEGMENTS
> - added the mixed-mode fix
> - completely removed GDT setup from efi_main, including for the 32-bit
> kernel
> - dropped documentation patches for now
>
> Arvind Sankar (7):
> x86/boot: Remove KEEP_SEGMENTS support
> efi/x86: Don't depend on firmware GDT layout
> x86/boot: Reload GDTR after copying to the end of the buffer
> x86/boot: Clear direction and interrupt flags in startup_64
> efi/x86: Remove GDT setup from efi_main
> x86/boot: GDT limit value should be size - 1
> x86/boot: Micro-optimize GDT loading instructions
>
> Documentation/x86/boot.rst | 8 +-
> arch/x86/boot/compressed/eboot.c | 103 ------------------------
> arch/x86/boot/compressed/efi_thunk_64.S | 29 +++++--
> arch/x86/boot/compressed/head_32.S | 48 +++++++----
> arch/x86/boot/compressed/head_64.S | 66 ++++++++-------
> arch/x86/kernel/head_32.S | 6 --
> 6 files changed, 99 insertions(+), 161 deletions(-)
>
> --
> 2.24.1
>