Received-SPF: pass (google.com: domain of linux-kernel+bounces-157681-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: "Eric W. Biederman" <ebiederm@xmission.com>
To: Ard Biesheuvel <ardb+git@google.com>
Cc: linux-kernel@vger.kernel.org,  x86@kernel.org,  Ard Biesheuvel
 <ardb@kernel.org>,  Arnd Bergmann <arnd@arndb.de>,
  kexec@lists.infradead.org,  Nathan Chancellor <nathan@kernel.org>,  Nick
 Desaulniers <ndesaulniers@google.com>,  Kees Cook <keescook@chromium.org>,
  Bill Wendling <morbo@google.com>,  Justin Stitt <justinstitt@google.com>,
  Masahiro Yamada <masahiroy@kernel.org>
References: <20240424155309.1719454-11-ardb+git@google.com>
Date: Wed, 24 Apr 2024 15:04:02 -0500
In-Reply-To: <20240424155309.1719454-11-ardb+git@google.com> (Ard Biesheuvel's
	message of "Wed, 24 Apr 2024 17:53:10 +0200")
Message-ID: <87bk5yv87h.fsf@email.froward.int.ebiederm.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [RFC PATCH 0/9] kexec x86 purgatory cleanup

Ard Biesheuvel <ardb+git@google.com> writes:

> From: Ard Biesheuvel <ardb@kernel.org>
>
> The kexec purgatory is built like a kernel module, i.e., a partially
> linked ELF object where each section is allocated and placed
> individually, and all relocations need to be fixed up, even place
> relative ones.
>
> This makes sense for kernel modules, which share the address space with
> the core kernel, and contain unresolved references that need to be wired
> up to symbols in other modules or the kernel itself.
>
> The purgatory, however, is a fully linked binary without any external
> references, or any overlap with the kernel's virtual address space. So
> it makes much more sense to create a fully linked ELF executable that
> can just be loaded and run anywhere in memory.

It does have external references that are resolved when it is loaded.

Further it is at least my impression that non-PIC code is more
efficient.  PIC typically requires silly things like Global Offset
Tables that non-PIC code does not.  At first glance this looks like a
code passivization.

Now at lot of functionality has been stripped out of purgatory so maybe
in it's stripped down this make sense, but I want to challenge the
notion that this is the obvious thing to do.

> The purgatory build on x86 has already switched over to position
> independent codegen, which only leaves a handful of absolute references,
> which can either be dropped (patch #3) or converted into a RIP-relative
> one (patch #4). That leaves a purgatory executable that can run at any
> offset in memory with applying any relocations whatsoever.

I missed that conversation.  Do you happen to have a pointer?  I would
think the 32bit code is where the PIC would be most costly as the 32bit
x86 instruction set predates PIC being a common compilation target.

> Some tweaks are needed to deal with the difference between partially
> (ET_REL) and fully (ET_DYN/ET_EXEC) linked ELF objects, but with those
> in place, a substantial amount of complicated ELF allocation, placement
> and patching/relocation code can simply be dropped.

Really?  As I recall it only needed to handle a single allocation type,
and there were good reasons (at least when I wrote it) to patch symbols.

Again maybe the fact that people have removed 90% of the functionality
makes this make sense, but that is not obvious at first glance.

> The last patch in the series removes this code from the generic kexec
> implementation, but this can only be done once other architectures apply
> the same changes proposed here for x86 (powerpc, s390 and riscv all
> implement the purgatory using the shared logic)
>
> Link: https://lore.kernel.org/all/CAKwvOd=3Jrzju++=Ve61=ZdeshxUM=K3-bGMNREnGOQgNw=aag@mail.gmail.com/
> Link: https://lore.kernel.org/all/20240418201705.3673200-2-ardb+git@google.com/
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Eric Biederman <ebiederm@xmission.com>
> Cc: kexec@lists.infradead.org
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Bill Wendling <morbo@google.com>
> Cc: Justin Stitt <justinstitt@google.com>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
>
> Ard Biesheuvel (9):
>   x86/purgatory: Drop function entry padding from purgatory
>   x86/purgatory: Simplify stack handling
>   x86/purgatory: Drop pointless GDT switch
>   x86/purgatory: Avoid absolute reference to GDT
>   x86/purgatory: Simplify GDT and drop data segment
>   kexec: Add support for fully linked purgatory executables
>   x86/purgatory: Use fully linked PIE ELF executable
>   x86/purgatory: Simplify references to regs array
>   kexec: Drop support for partially linked purgatory executables
>
>  arch/x86/include/asm/kexec.h       |   8 -
>  arch/x86/kernel/kexec-bzimage64.c  |   8 -
>  arch/x86/kernel/machine_kexec_64.c | 127 ----------
>  arch/x86/purgatory/Makefile        |  17 +-
>  arch/x86/purgatory/entry64.S       |  96 ++++----
>  arch/x86/purgatory/setup-x86_64.S  |  31 +--
>  arch/x86/purgatory/stack.S         |  18 --
>  include/asm-generic/purgatory.lds  |  34 +++
>  kernel/kexec_file.c                | 255 +++-----------------
>  9 files changed, 125 insertions(+), 469 deletions(-)
>  delete mode 100644 arch/x86/purgatory/stack.S
>  create mode 100644 include/asm-generic/purgatory.lds

Eric