Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200130200440.1796058-1-nivedita@alum.mit.edu> <20200202171353.3736319-1-nivedita@alum.mit.edu>
In-Reply-To: <20200202171353.3736319-1-nivedita@alum.mit.edu>
From:   Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date:   Sun, 2 Feb 2020 19:01:27 +0100
Message-ID: <CAKv+Gu_pXg3hFophgQZ2nMHMWZwUuiE2s+Z6bRzby8_3vXzcNg@mail.gmail.com>
Subject: Re: [PATCH v2 0/7] x86/efi,boot: GDT handling cleanup/fixes
To:     Arvind Sankar <nivedita@alum.mit.edu>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        "H. Peter Anvin" <hpa@zytor.com>, Ard Biesheuvel <ardb@kernel.org>,
        linux-efi <linux-efi@vger.kernel.org>,
        "the arch/x86 maintainers" <x86@kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sun, 2 Feb 2020 at 18:13, Arvind Sankar <nivedita@alum.mit.edu> 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@kernel.org>

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
>