Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230416120729.2470762-1-ardb@kernel.org> <5694ea5d-da9a-413e-9499-02a54588a953@app.fastmail.com>
 <CAMj1kXH5+scbFuaOP+VC7EHEZcn-tmp3nk=9uYGYGfJyb0S92Q@mail.gmail.com> <44b21bb3-a65d-4560-9fe5-c7ddc260ddc0@app.fastmail.com>
In-Reply-To: <44b21bb3-a65d-4560-9fe5-c7ddc260ddc0@app.fastmail.com>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Wed, 3 May 2023 20:13:28 +0200
Message-ID: <CAMj1kXHiw2_7HAJND4HryTRqbTLoucyC4yATPXOTUnA1tkeyzA@mail.gmail.com>
Subject: Re: [RFC PATCH 0/3] efi: Implement generic zboot support
To:     Andy Lutomirski <luto@kernel.org>
Cc:     linux-efi@vger.kernel.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Evgeniy Baskov <baskov@ispras.ru>,
        Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Ingo Molnar <mingo@redhat.com>,
        "Peter Zijlstra (Intel)" <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Alexey Khoroshilov <khoroshilov@ispras.ru>,
        Peter Jones <pjones@redhat.com>,
        Gerd Hoffmann <kraxel@redhat.com>,
        Dave Young <dyoung@redhat.com>,
        Mario Limonciello <mario.limonciello@amd.com>,
        Kees Cook <keescook@chromium.org>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
        Linus Torvalds <torvalds@linux-foundation.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, 3 May 2023 at 19:55, Andy Lutomirski <luto@kernel.org> wrote:
>
> On Fri, Apr 21, 2023, at 6:41 AM, Ard Biesheuvel wrote:
> > On Fri, 21 Apr 2023 at 15:30, Andy Lutomirski <luto@kernel.org> wrote:
> >>
> >>
> >>
> >> On Sun, Apr 16, 2023, at 5:07 AM, Ard Biesheuvel wrote:
> >> > This series is a proof-of-concept that implements support for the EF=
I
> >> > zboot decompressor for x86. It replaces the ordinary decompressor, a=
nd
> >> > instead, performs the decompression, KASLR randomization and the 4/5
> >> > level paging switch while running in the execution context of EFI.
> >>
> >> I like the concept.  A couple high-level questions, since I haven=E2=
=80=99t dug into the code:
> >>
> >> Could zboot and bzImage be built into the same kernel image?  That wou=
ld get this into distros, and eventually someone could modify the legacy pa=
th to switch to long mode and invoke zboot (because zboot surely doesn=E2=
=80=99t need actual UEFI =E2=80=94 just a sensible environment like what UE=
FI provides.)
> >>
> >
> > That's an interesting question, and to some extent, that is actually
> > what Evgeny's patch does: execute more of what the decompressor does
> > from inside the EFI runtime context.
> >
> > The main win with zboot imho is that we get rid of all the funky
> > heuristics that look for usable memory for trampolines and
> > decompression buffers in funky ways, and instead, just use the EFI
> > APIs for allocating pages and remapping them executable as needed
> > (which is the important piece here) I'd have to think about whether
> > there is any middle ground between this approach and Evgeny's - I'll
> > have to get back to you on that.
> >
>
> Hmm.  I dug the tiniest bit into the history.  The x86/boot/compressed st=
uff has an allocator!  It's this:
>
>         free_mem_ptr     =3D heap;        /* Heap */
>         free_mem_end_ptr =3D heap + BOOT_HEAP_SIZE;
>
> plus a trivial and horrible malloc() implementation in include/linux/deco=
mpress/mm.h.  There's one caller in x86/boot/compressed.
>
> And, once upon a time, the idea of allocating enough memory to store the =
kernel from the decompressor would have been a problem.  I'm willing to cla=
im that we should not even try to support x86 systems that have that little=
 memory (at least not once they've gotten long mode or at least flat 32-bit=
 protected mode working).  We should not try to allocate below 1MB (my lapt=
op will cry), but there's no need for that.
>
> So maybe the middle ground is to build a modern, simple malloc(), and bac=
k it by EFI when EFI is there and by just finding some free memory when EFI=
 is not there?
>
> This would be risky -- someone might have a horrible machine that has tro=
uble with a simple allocator.

The malloc() is the least or our concerns, tbh. It is only used by the
decompression library itself, and it is backed by a statically
allocated block of BSS.

Having just gone through this again, the main issues are:

1) The 4/5 level switching trampoline, which runs in the page tables
of the loader/EFI stub, and assumes that it is fine to grab a random
chunk of low memory, stash its contents somewhere, use it for some
code, a stack and a root level page table so we can do the x86 long
mode paging off/paging on salsa, and then copy back the contents and
carry on as if nothing happened. We currently have some code in the
stub that strips all NX restrictions from a generously overdimensioned
block of low memory so copying and running code like this actually
works.

2) We need an accurate description in the PE/COFF header of what needs
to be executable and what needs to be writable, so we can splt the
regions. This only matters for code that runs under EFI's mappings, so
not a lot.

3) The payload relocates itself to the end of the decompression buffer
so it doesn't overwrite itself before completing. This is fragile and
also unnecessary when there is a page allocator and plenty of memory,
but afaict, this all executes under the decompressor's own page tables
so the RO/NX attributes that EFI sets are not a concern here. It
would, of course, be nice if we could avoid relying on RWX mappings
here.

4) I think it was you who pointed out that the demand paging 1:1 map
should really only get triggered for data accesses and not code
accesses, so it would be nice if we could create such mappings with NX
attributes.

I've had another go at running the decompressed kernel directly,
without going through the decompressor logic at all, but I missed the
fact that SEV does a substantial amount of work in the decompressor
too, so I'm no longer convinced that this is a viable approach. But
I'm looking into this.

I just finished some patches [0] that only address 1), based on the
work I posted earlier. I'll send those out once -rc1 comes around.


[0] https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=
=3Defi-x86-cleanup-la57