linux-efi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL v2] EFI updates for v5.7
@ 2020-02-24 13:38 Ard Biesheuvel
  2020-02-26 14:27 ` Ingo Molnar
  0 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2020-02-24 13:38 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner; +Cc: x86, Ard Biesheuvel, linux-kernel

Hello Ingo, Thomas,

I am sending this as an ordinary PR again, given the size. Please let me
know if instead, you prefer me to send it out piecemeal as usual. Either
works for me, I was just reluctant to spam people unsolicited.

Note that EFI for RISC-V may still arrive this cycle as well. 

Please take special note of the GDT changes by Arvind. They were posted to
the list without any feedback, and they look fine to me, but I know very
little about these x86 CPU low level details.

This was all build and boot tested on various different kinds of hardware,
and all minor issues that were reported by the robots were fixed along the way.

Please pull,
Ard.


The following changes since commit bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9:

  Linux 5.6-rc1 (2020-02-09 16:08:48 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git tags/efi-next

for you to fetch changes up to dc235d62fc60a6549238eda7ff29769457fe5663:

  efi: Bump the Linux EFI stub major version number to #1 (2020-02-23 21:59:42 +0100)

----------------------------------------------------------------
EFI updates for v5.7:

This time, the set of changes for the EFI subsystem is much larger than
usual. The main reasons are:
- Get things cleaned up before EFI support for RISC-V arrives, which will
  increase the size of the validation matrix, and therefore the threshold to
  making drastic changes,
- After years of defunct maintainership, the GRUB project has finally started
  to consider changes from the distros regarding UEFI boot, some of which are
  highly specific to the way x86 does UEFI secure boot and measured boot,
  based on knowledge of both shim internals and the layout of bootparams and
  the x86 setup header. Having this maintenance burden on other architectures
  (which don't need shim in the first place) is hard to justify, so instead,
  we are introducing a generic Linux/UEFI boot protocol.

Summary of changes:
- Boot time GDT handling changes (Arvind)
- Simplify handling of EFI properties table on arm64
- Generic EFI stub cleanups, to improve command line handling, file I/O,
  memory allocation, etc.
- Introduce a generic initrd loading method based on calling back into
  the firmware, instead of relying on the x86 EFI handover protocol or
  device tree.
- Introduce a mixed mode boot method that does not rely on the x86 EFI
  handover protocol either, and could potentially be adopted by other
  architectures (if another one ever surfaces where one execution mode
  is a superset of another)
- Clean up the contents of struct efi, and move out everything that
  doesn't need to be stored there.
- Incorporate support for UEFI spec v2.8A changes that permit firmware
  implementations to return EFI_UNSUPPORTED from UEFI runtime services at
  OS runtime, and expose a mask of which ones are supported or unsupported
  via a configuration table.
- Various documentation updates and minor code cleanups (Heinrich)
- Partial fix for the lack of by-VA cache maintenance in the decompressor
  on 32-bit ARM. Note that these patches were deliberately put at the
  beginning so they can be used as a stable branch that will be shared with
  a PR containing the complete fix, which I will send to the ARM tree.

----------------------------------------------------------------
Ard Biesheuvel (67):
      efi/arm: Work around missing cache maintenance in decompressor handover
      efi/arm: Pass start and end addresses to cache_clean_flush()
      efi/libstub/arm: Make efi_entry() an ordinary PE/COFF entrypoint
      efi/libstub/arm64: Use 1:1 mapping of RT services if property table exists
      efi/libstub/x86: Remove pointless zeroing of apm_bios_info
      efi/libstub/x86: Avoid overflowing code32_start on PE entry
      efi/libstub: Use hidden visibility for all source files
      efi/libstub/arm: Relax FDT alignment requirement
      efi/libstub: Move memory map handling and allocation routines to mem.c
      efi/libstub: Simplify efi_high_alloc() and rename to efi_allocate_pages()
      efi/libstub/x86: Incorporate eboot.c into libstub
      efi/libstub: Use consistent type names for file I/O protocols
      efi/libstub/x86: Permit bootparams struct to be allocated above 4 GB
      efi/libstub: Move stub specific declarations into efistub.h
      efi/libstub/x86: Permit cmdline data to be allocated above 4 GB
      efi/libstub: Move efi_random_alloc() into separate source file
      efi/libstub: Move get_dram_base() into arm-stub.c
      efi/libstub: Move file I/O support code into separate file
      efi/libstub: Rewrite file I/O routine
      efi/libstub: Take soft and hard memory limits into account for initrd loading
      efi/libstub: Clean up command line parsing routine
      efi/libstub: Expose LocateDevicePath boot service
      efi/libstub: Make the LoadFile EFI protocol accessible
      efi/x86: Reindent struct initializer for legibility
      efi/x86: Replace #ifdefs with IS_ENABLED() checks
      efi/dev-path-parser: Add struct definition for vendor type device path nodes
      efi/libstub: Add support for loading the initrd from a device path
      efi/libstub: Take noinitrd cmdline argument into account for devpath initrd
      efi: Drop handling of 'boot_info' configuration table
      efi/ia64: Move HCDP and MPS table handling into IA64 arch code
      efi: Move UGA and PROP table handling to x86 code
      efi: Make rng_seed table handling local to efi.c
      efi: Move mem_attr_table out of struct efi
      efi: Make memreserve table handling local to efi.c
      efi: Merge EFI system table revision and vendor checks
      efi/ia64: Use existing helpers to locate ESI table
      efi/ia64: Use local variable for EFI system table address
      efi/ia64: Switch to efi_config_parse_tables()
      efi: Make efi_config_init() x86 only
      efi: Clean up config_parse_tables()
      efi/x86: Remove runtime table address from kexec EFI setup data
      efi/x86: Make fw_vendor, config_table and runtime sysfs nodes x86 specific
      efi/x86: Merge assignments of efi.runtime_version
      efi: Add 'runtime' pointer to struct efi
      efi/arm: Drop unnecessary references to efi.systab
      efi/x86: Drop 'systab' member from struct efi
      efi/x86: add headroom to decompressor BSS to account for setup block
      efi/x86: Drop redundant .bss section
      efi/libstub/x86: Make loaded_image protocol handling mixed mode safe
      efi/libstub/x86: Use Exit() boot service to exit the stub on errors
      efi/x86: Implement mixed mode boot without the handover protocol
      efi/x86: Add true mixed mode entry point into .compat section
      efi/arm: Move FDT param discovery code out of efi.c
      efi/arm: Move FDT specific definitions into fdtparams.c
      efi/arm: Rewrite FDT param discovery routines
      efi: Store mask of supported runtime services in struct efi
      efi: Add support for EFI_RT_PROPERTIES table
      efi: Use more granular check for availability for variable services
      efi: Register EFI rtc platform device only when available
      infiniband: hfi1: Use EFI GetVariable only when available
      scsi: iscsi: Use EFI GetVariable only when available
      efi: Use EFI ResetSystem only when available
      x86/ima: Use EFI GetVariable only when available
      integrity: Check properly whether EFI GetVariable() is available
      efi/x86: Use symbolic constants in PE header instead of bare numbers
      efi/libstub: Introduce symbolic constants for the stub major/minor version
      efi: Bump the Linux EFI stub major version number to #1

Arvind Sankar (8):
      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
      efi/x86: Mark setup_graphics static

Gustavo A. R. Silva (1):
      efi/apple-properties: Replace zero-length array with flexible-array member

Hans de Goede (1):
      efi/bgrt: Accept BGRT tables with a version of 0

Heinrich Schuchardt (8):
      efi/libstub: Add function description of efi_allocate_pages()
      efi/libstub: Simplify efi_get_memory_map()
      efi/libstub: Describe memory functions
      efi/libstub: Describe efi_relocate_kernel()
      efi/libstub: Describe RNG functions
      efi/libstub: Fix error message in handle_cmdline_files()
      efi/esrt: Clean up efi_esrt_init
      efi/capsule-loader: Drop superfluous assignment

 Documentation/x86/boot.rst                         |   8 +-
 arch/arm/boot/compressed/efi-header.S              |   6 +-
 arch/arm/boot/compressed/head.S                    |  60 +-
 arch/arm64/include/asm/efi.h                       |  10 -
 arch/arm64/kernel/efi-entry.S                      |  86 +--
 arch/arm64/kernel/efi-header.S                     |   6 +-
 arch/arm64/kernel/image-vars.h                     |   5 +-
 arch/ia64/kernel/efi.c                             |  55 +-
 arch/ia64/kernel/esi.c                             |  21 +-
 arch/x86/boot/Makefile                             |   2 +-
 arch/x86/boot/compressed/Makefile                  |   5 +-
 arch/x86/boot/compressed/eboot.h                   |  31 -
 arch/x86/boot/compressed/efi_thunk_64.S            |  29 +-
 arch/x86/boot/compressed/head_32.S                 |  48 +-
 arch/x86/boot/compressed/head_64.S                 | 125 +++-
 arch/x86/boot/header.S                             |  87 +--
 arch/x86/boot/tools/build.c                        |  86 ++-
 arch/x86/include/asm/efi.h                         |  23 +-
 arch/x86/kernel/asm-offsets_32.c                   |   5 +
 arch/x86/kernel/head_32.S                          |   6 -
 arch/x86/kernel/ima_arch.c                         |   2 +-
 arch/x86/kernel/kexec-bzimage64.c                  |   5 +-
 arch/x86/platform/efi/efi.c                        | 283 ++++---
 arch/x86/platform/efi/efi_32.c                     |  13 +-
 arch/x86/platform/efi/efi_64.c                     |  14 +-
 arch/x86/platform/efi/efi_stub_32.S                |  21 +-
 arch/x86/platform/efi/quirks.c                     |   2 +-
 drivers/firmware/efi/Makefile                      |   1 +
 drivers/firmware/efi/apple-properties.c            |  12 +-
 drivers/firmware/efi/arm-init.c                    |  83 +--
 drivers/firmware/efi/arm-runtime.c                 |  18 -
 drivers/firmware/efi/capsule-loader.c              |   2 +-
 drivers/firmware/efi/dev-path-parser.c             |  38 +-
 drivers/firmware/efi/efi-bgrt.c                    |   7 +-
 drivers/firmware/efi/efi-pstore.c                  |   2 +-
 drivers/firmware/efi/efi.c                         | 418 ++++-------
 drivers/firmware/efi/efivars.c                     |   2 +-
 drivers/firmware/efi/esrt.c                        |   6 +-
 drivers/firmware/efi/fdtparams.c                   | 126 ++++
 drivers/firmware/efi/libstub/Makefile              |   6 +-
 drivers/firmware/efi/libstub/arm-stub.c            | 193 ++---
 drivers/firmware/efi/libstub/arm32-stub.c          |   1 +
 drivers/firmware/efi/libstub/arm64-stub.c          |  11 +-
 drivers/firmware/efi/libstub/efi-stub-helper.c     | 822 ++++-----------------
 drivers/firmware/efi/libstub/efistub.h             | 611 ++++++++++++++-
 drivers/firmware/efi/libstub/fdt.c                 |   7 +-
 drivers/firmware/efi/libstub/file.c                | 258 +++++++
 drivers/firmware/efi/libstub/hidden.h              |   6 +
 drivers/firmware/efi/libstub/mem.c                 | 309 ++++++++
 drivers/firmware/efi/libstub/random.c              | 136 +---
 drivers/firmware/efi/libstub/randomalloc.c         | 124 ++++
 drivers/firmware/efi/libstub/skip_spaces.c         |  11 +
 drivers/firmware/efi/libstub/string.c              |  56 ++
 .../firmware/efi/libstub/x86-stub.c                | 258 +++----
 drivers/firmware/efi/memattr.c                     |  13 +-
 drivers/firmware/efi/reboot.c                      |   4 +-
 drivers/firmware/efi/runtime-wrappers.c            |   4 +-
 drivers/firmware/pcdp.c                            |   8 +-
 drivers/infiniband/hw/hfi1/efivar.c                |   2 +-
 drivers/rtc/Makefile                               |   4 -
 drivers/rtc/rtc-efi-platform.c                     |  35 -
 drivers/scsi/isci/init.c                           |   2 +-
 fs/efivarfs/super.c                                |   2 +-
 include/linux/efi.h                                | 691 +++--------------
 include/linux/pe.h                                 |  21 +
 security/integrity/platform_certs/load_uefi.c      |   2 +-
 66 files changed, 2718 insertions(+), 2638 deletions(-)
 delete mode 100644 arch/x86/boot/compressed/eboot.h
 create mode 100644 drivers/firmware/efi/fdtparams.c
 create mode 100644 drivers/firmware/efi/libstub/file.c
 create mode 100644 drivers/firmware/efi/libstub/hidden.h
 create mode 100644 drivers/firmware/efi/libstub/mem.c
 create mode 100644 drivers/firmware/efi/libstub/randomalloc.c
 create mode 100644 drivers/firmware/efi/libstub/skip_spaces.c
 rename arch/x86/boot/compressed/eboot.c => drivers/firmware/efi/libstub/x86-stub.c (82%)
 delete mode 100644 drivers/rtc/rtc-efi-platform.c

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [GIT PULL v2] EFI updates for v5.7
  2020-02-24 13:38 [GIT PULL v2] EFI updates for v5.7 Ard Biesheuvel
@ 2020-02-26 14:27 ` Ingo Molnar
  2020-02-26 14:50   ` Ard Biesheuvel
                     ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Ingo Molnar @ 2020-02-26 14:27 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi, Thomas Gleixner, x86, linux-kernel, Arvind Sankar


* Ard Biesheuvel <ardb@kernel.org> wrote:

> Hello Ingo, Thomas,
> 
> I am sending this as an ordinary PR again, given the size. Please let me
> know if instead, you prefer me to send it out piecemeal as usual. Either
> works for me, I was just reluctant to spam people unsolicited.
> 
> Note that EFI for RISC-V may still arrive this cycle as well. 
> 
> Please take special note of the GDT changes by Arvind. They were posted to
> the list without any feedback, and they look fine to me, but I know very
> little about these x86 CPU low level details.
> 
> This was all build and boot tested on various different kinds of hardware,
> and all minor issues that were reported by the robots were fixed along the way.
> 
> Please pull,
> Ard.
> 
> 
> The following changes since commit bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9:
> 
>   Linux 5.6-rc1 (2020-02-09 16:08:48 -0800)
> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git tags/efi-next
> 
> for you to fetch changes up to dc235d62fc60a6549238eda7ff29769457fe5663:
> 
>   efi: Bump the Linux EFI stub major version number to #1 (2020-02-23 21:59:42 +0100)

>  66 files changed, 2718 insertions(+), 2638 deletions(-)

Pulled this as a Git tree, thanks Ard!

The boot-time GDT handling cleanups from Arvind look good to me too. 
There's a couple of arguably scary commits in there, so these might be 
'famous last words'. :-)

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [GIT PULL v2] EFI updates for v5.7
  2020-02-26 14:27 ` Ingo Molnar
@ 2020-02-26 14:50   ` Ard Biesheuvel
  2020-02-26 20:45   ` [PATCH 0/1] Small fix to boot-time GDT handling Arvind Sankar
  2020-02-26 20:45   ` [PATCH 1/1] x86/boot/compressed/32: " Arvind Sankar
  2 siblings, 0 replies; 15+ messages in thread
From: Ard Biesheuvel @ 2020-02-26 14:50 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-efi, Thomas Gleixner, the arch/x86 maintainers,
	Linux Kernel Mailing List, Arvind Sankar

On Wed, 26 Feb 2020 at 15:27, Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Ard Biesheuvel <ardb@kernel.org> wrote:
>
> > Hello Ingo, Thomas,
> >
> > I am sending this as an ordinary PR again, given the size. Please let me
> > know if instead, you prefer me to send it out piecemeal as usual. Either
> > works for me, I was just reluctant to spam people unsolicited.
> >
> > Note that EFI for RISC-V may still arrive this cycle as well.
> >
> > Please take special note of the GDT changes by Arvind. They were posted to
> > the list without any feedback, and they look fine to me, but I know very
> > little about these x86 CPU low level details.
> >
> > This was all build and boot tested on various different kinds of hardware,
> > and all minor issues that were reported by the robots were fixed along the way.
> >
> > Please pull,
> > Ard.
> >
> >
> > The following changes since commit bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9:
> >
> >   Linux 5.6-rc1 (2020-02-09 16:08:48 -0800)
> >
> > are available in the Git repository at:
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git tags/efi-next
> >
> > for you to fetch changes up to dc235d62fc60a6549238eda7ff29769457fe5663:
> >
> >   efi: Bump the Linux EFI stub major version number to #1 (2020-02-23 21:59:42 +0100)
>
> >  66 files changed, 2718 insertions(+), 2638 deletions(-)
>
> Pulled this as a Git tree, thanks Ard!
>
> The boot-time GDT handling cleanups from Arvind look good to me too.
> There's a couple of arguably scary commits in there, so these might be
> 'famous last words'. :-)
>

Thanks!

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 0/1] Small fix to boot-time GDT handling
  2020-02-26 14:27 ` Ingo Molnar
  2020-02-26 14:50   ` Ard Biesheuvel
@ 2020-02-26 20:45   ` Arvind Sankar
  2020-02-26 23:00     ` [PATCH v2 " Arvind Sankar
  2020-02-26 23:00     ` [PATCH v2 1/1] x86/boot/compressed: Fix reloading of GDTR post-relocation Arvind Sankar
  2020-02-26 20:45   ` [PATCH 1/1] x86/boot/compressed/32: " Arvind Sankar
  2 siblings, 2 replies; 15+ messages in thread
From: Arvind Sankar @ 2020-02-26 20:45 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Ard Biesheuvel, linux-efi, Thomas Gleixner, x86, linux-kernel

Hi Ingo, I noticed a potential problem with the 32-bit GDT handling code
that I had submitted. This patch (based on tip:efi/core) should address
that.

Thanks.

Arvind Sankar (1):
  x86/boot/compressed/32: Fix reloading of GDTR post-relocation

 arch/x86/boot/compressed/head_32.S | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

-- 
2.24.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/1] x86/boot/compressed/32: Fix reloading of GDTR post-relocation
  2020-02-26 14:27 ` Ingo Molnar
  2020-02-26 14:50   ` Ard Biesheuvel
  2020-02-26 20:45   ` [PATCH 0/1] Small fix to boot-time GDT handling Arvind Sankar
@ 2020-02-26 20:45   ` Arvind Sankar
  2 siblings, 0 replies; 15+ messages in thread
From: Arvind Sankar @ 2020-02-26 20:45 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Ard Biesheuvel, linux-efi, Thomas Gleixner, x86, linux-kernel

Commit ef5a7b5eb13e ("efi/x86: Remove GDT setup from efi_main")
introduced GDT setup into startup_32, and reloads the GDTR after
relocating the kernel for paranoia's sake.

The GDTR is adjusted by init_size - _end, however this may not be the
correct offset to apply if the kernel was loaded at a misaligned address
or below LOAD_PHYSICAL_ADDR, as in that case the decompression buffer
has an additional offset from the original load address.

This should never happen for a conformant bootloader, but we're being
paranoid anyway, so just store the new GDT address in there instead of
adding any offsets, which is simpler as well.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
Fixes: ef5a7b5eb13e ("efi/x86: Remove GDT setup from efi_main")
---
 arch/x86/boot/compressed/head_32.S | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index 356060c5332c..2f8138b71ea9 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -139,12 +139,11 @@ SYM_FUNC_START(startup_32)
 	/*
 	 * The GDT may get overwritten either during the copy we just did or
 	 * during extract_kernel below. To avoid any issues, repoint the GDTR
-	 * to the new copy of the GDT. EAX still contains the previously
-	 * calculated relocation offset of init_size - _end.
+	 * to the new copy of the GDT.
 	 */
-	leal	gdt(%ebx), %edx
-	addl	%eax, 2(%edx)
-	lgdt	(%edx)
+	leal	gdt(%ebx), %eax
+	movl	%eax, 2(%eax)
+	lgdt	(%eax)
 
 /*
  * Jump to the relocated address.
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 0/1] Small fix to boot-time GDT handling
  2020-02-26 20:45   ` [PATCH 0/1] Small fix to boot-time GDT handling Arvind Sankar
@ 2020-02-26 23:00     ` Arvind Sankar
  2020-02-26 23:00     ` [PATCH v2 1/1] x86/boot/compressed: Fix reloading of GDTR post-relocation Arvind Sankar
  1 sibling, 0 replies; 15+ messages in thread
From: Arvind Sankar @ 2020-02-26 23:00 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Ard Biesheuvel, linux-efi, Thomas Gleixner, x86, linux-kernel

Hi Ingo, I noticed a potential problem with the GDT handling code that I
had submitted. This patch (based on tip:efi/core) should address that.

Thanks.

Changes from v1: The 64-bit version had the same problem, fix that too.

Arvind Sankar (1):
  x86/boot/compressed: Fix reloading of GDTR post-relocation

 arch/x86/boot/compressed/head_32.S | 9 ++++-----
 arch/x86/boot/compressed/head_64.S | 4 ++--
 2 files changed, 6 insertions(+), 7 deletions(-)

-- 
2.24.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/1] x86/boot/compressed: Fix reloading of GDTR post-relocation
  2020-02-26 20:45   ` [PATCH 0/1] Small fix to boot-time GDT handling Arvind Sankar
  2020-02-26 23:00     ` [PATCH v2 " Arvind Sankar
@ 2020-02-26 23:00     ` Arvind Sankar
  2020-02-27  8:12       ` Ingo Molnar
  1 sibling, 1 reply; 15+ messages in thread
From: Arvind Sankar @ 2020-02-26 23:00 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Ard Biesheuvel, linux-efi, Thomas Gleixner, x86, linux-kernel

Commit ef5a7b5eb13e ("efi/x86: Remove GDT setup from efi_main")
introduced GDT setup into the 32-bit kernel's startup_32, and reloads
the GDTR after relocating the kernel for paranoia's sake.

Commit 32d009137a56 ("x86/boot: Reload GDTR after copying to the end of
the buffer") introduced a similar GDTR reload in the 64-bit kernel.

The GDTR is adjusted by init_size - _end, however this may not be the
correct offset to apply if the kernel was loaded at a misaligned address
or below LOAD_PHYSICAL_ADDR, as in that case the decompression buffer
has an additional offset from the original load address.

This should never happen for a conformant bootloader, but we're being
paranoid anyway, so just store the new GDT address in there instead of
adding any offsets, which is simpler as well.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
Fixes: ef5a7b5eb13e ("efi/x86: Remove GDT setup from efi_main")
Fixes: 32d009137a56 ("x86/boot: Reload GDTR after copying to the end of the buffer")
---
 arch/x86/boot/compressed/head_32.S | 9 ++++-----
 arch/x86/boot/compressed/head_64.S | 4 ++--
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index 356060c5332c..2f8138b71ea9 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -139,12 +139,11 @@ SYM_FUNC_START(startup_32)
 	/*
 	 * The GDT may get overwritten either during the copy we just did or
 	 * during extract_kernel below. To avoid any issues, repoint the GDTR
-	 * to the new copy of the GDT. EAX still contains the previously
-	 * calculated relocation offset of init_size - _end.
+	 * to the new copy of the GDT.
 	 */
-	leal	gdt(%ebx), %edx
-	addl	%eax, 2(%edx)
-	lgdt	(%edx)
+	leal	gdt(%ebx), %eax
+	movl	%eax, 2(%eax)
+	lgdt	(%eax)
 
 /*
  * Jump to the relocated address.
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index f7bacc4c1494..fcf8feaa57ea 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -456,8 +456,8 @@ trampoline_return:
 	 * to the new copy of the GDT.
 	 */
 	leaq	gdt64(%rbx), %rax
-	subq	%rbp, 2(%rax)
-	addq	%rbx, 2(%rax)
+	leaq	gdt(%rbx), %rdx
+	movq	%rdx, 2(%rax)
 	lgdt	(%rax)
 
 /*
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/1] x86/boot/compressed: Fix reloading of GDTR post-relocation
  2020-02-26 23:00     ` [PATCH v2 1/1] x86/boot/compressed: Fix reloading of GDTR post-relocation Arvind Sankar
@ 2020-02-27  8:12       ` Ingo Molnar
  2020-02-27 15:16         ` Arvind Sankar
  0 siblings, 1 reply; 15+ messages in thread
From: Ingo Molnar @ 2020-02-27  8:12 UTC (permalink / raw)
  To: Arvind Sankar
  Cc: Ard Biesheuvel, linux-efi, Thomas Gleixner, x86, linux-kernel,
	Borislav Petkov


* Arvind Sankar <nivedita@alum.mit.edu> wrote:

> Commit ef5a7b5eb13e ("efi/x86: Remove GDT setup from efi_main")
> introduced GDT setup into the 32-bit kernel's startup_32, and reloads
> the GDTR after relocating the kernel for paranoia's sake.
> 
> Commit 32d009137a56 ("x86/boot: Reload GDTR after copying to the end of
> the buffer") introduced a similar GDTR reload in the 64-bit kernel.
> 
> The GDTR is adjusted by init_size - _end, however this may not be the
> correct offset to apply if the kernel was loaded at a misaligned address
> or below LOAD_PHYSICAL_ADDR, as in that case the decompression buffer
> has an additional offset from the original load address.
> 
> This should never happen for a conformant bootloader, but we're being
> paranoid anyway, so just store the new GDT address in there instead of
> adding any offsets, which is simpler as well.
> 
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> Fixes: ef5a7b5eb13e ("efi/x86: Remove GDT setup from efi_main")
> Fixes: 32d009137a56 ("x86/boot: Reload GDTR after copying to the end of the buffer")

Have you or anyone else observed this condition practice, or have a 
suspicion that this could happen - or is this a mostly theoretical 
concern?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/1] x86/boot/compressed: Fix reloading of GDTR post-relocation
  2020-02-27  8:12       ` Ingo Molnar
@ 2020-02-27 15:16         ` Arvind Sankar
  2020-02-27 15:21           ` Ard Biesheuvel
  0 siblings, 1 reply; 15+ messages in thread
From: Arvind Sankar @ 2020-02-27 15:16 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Arvind Sankar, Ard Biesheuvel, linux-efi, Thomas Gleixner, x86,
	linux-kernel, Borislav Petkov

On Thu, Feb 27, 2020 at 09:12:29AM +0100, Ingo Molnar wrote:
> 
> * Arvind Sankar <nivedita@alum.mit.edu> wrote:
> 
> > Commit ef5a7b5eb13e ("efi/x86: Remove GDT setup from efi_main")
> > introduced GDT setup into the 32-bit kernel's startup_32, and reloads
> > the GDTR after relocating the kernel for paranoia's sake.
> > 
> > Commit 32d009137a56 ("x86/boot: Reload GDTR after copying to the end of
> > the buffer") introduced a similar GDTR reload in the 64-bit kernel.
> > 
> > The GDTR is adjusted by init_size - _end, however this may not be the
> > correct offset to apply if the kernel was loaded at a misaligned address
> > or below LOAD_PHYSICAL_ADDR, as in that case the decompression buffer
> > has an additional offset from the original load address.
> > 
> > This should never happen for a conformant bootloader, but we're being
> > paranoid anyway, so just store the new GDT address in there instead of
> > adding any offsets, which is simpler as well.
> > 
> > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> > Fixes: ef5a7b5eb13e ("efi/x86: Remove GDT setup from efi_main")
> > Fixes: 32d009137a56 ("x86/boot: Reload GDTR after copying to the end of the buffer")
> 
> Have you or anyone else observed this condition practice, or have a 
> suspicion that this could happen - or is this a mostly theoretical 
> concern?
> 
> Thanks,
> 
> 	Ingo

Right now it's a theoretical concern.

I'm working on another patch, to tell the EFI firmware PE loader what
the kernel's preferred address is, so that we can avoid having to
relocate the kernel in the EFI stub in most cases (ie if the PE loader
manages to load us at that address). With those changes, the required
adjustment won't be init_size - _end any more, and while fixing it up
there, I noticed that it could already be the case that the required
adjustment is different.

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/1] x86/boot/compressed: Fix reloading of GDTR post-relocation
  2020-02-27 15:16         ` Arvind Sankar
@ 2020-02-27 15:21           ` Ard Biesheuvel
  2020-02-27 15:54             ` Arvind Sankar
  0 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2020-02-27 15:21 UTC (permalink / raw)
  To: Arvind Sankar
  Cc: Ingo Molnar, linux-efi, Thomas Gleixner,
	the arch/x86 maintainers, Linux Kernel Mailing List,
	Borislav Petkov

On Thu, 27 Feb 2020 at 16:16, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Thu, Feb 27, 2020 at 09:12:29AM +0100, Ingo Molnar wrote:
> >
> > * Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > > Commit ef5a7b5eb13e ("efi/x86: Remove GDT setup from efi_main")
> > > introduced GDT setup into the 32-bit kernel's startup_32, and reloads
> > > the GDTR after relocating the kernel for paranoia's sake.
> > >
> > > Commit 32d009137a56 ("x86/boot: Reload GDTR after copying to the end of
> > > the buffer") introduced a similar GDTR reload in the 64-bit kernel.
> > >
> > > The GDTR is adjusted by init_size - _end, however this may not be the
> > > correct offset to apply if the kernel was loaded at a misaligned address
> > > or below LOAD_PHYSICAL_ADDR, as in that case the decompression buffer
> > > has an additional offset from the original load address.
> > >
> > > This should never happen for a conformant bootloader, but we're being
> > > paranoid anyway, so just store the new GDT address in there instead of
> > > adding any offsets, which is simpler as well.
> > >
> > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > Fixes: ef5a7b5eb13e ("efi/x86: Remove GDT setup from efi_main")
> > > Fixes: 32d009137a56 ("x86/boot: Reload GDTR after copying to the end of the buffer")
> >
> > Have you or anyone else observed this condition practice, or have a
> > suspicion that this could happen - or is this a mostly theoretical
> > concern?
> >
> > Thanks,
> >
> >       Ingo
>
> Right now it's a theoretical concern.
>
> I'm working on another patch, to tell the EFI firmware PE loader what
> the kernel's preferred address is, so that we can avoid having to
> relocate the kernel in the EFI stub in most cases (ie if the PE loader
> manages to load us at that address). With those changes, the required
> adjustment won't be init_size - _end any more, and while fixing it up
> there, I noticed that it could already be the case that the required
> adjustment is different.
>

Do you mean setting the image address in the PE/COFF header to the
preferred address?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/1] x86/boot/compressed: Fix reloading of GDTR post-relocation
  2020-02-27 15:21           ` Ard Biesheuvel
@ 2020-02-27 15:54             ` Arvind Sankar
  2020-02-27 17:47               ` Ard Biesheuvel
  0 siblings, 1 reply; 15+ messages in thread
From: Arvind Sankar @ 2020-02-27 15:54 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Arvind Sankar, Ingo Molnar, linux-efi, Thomas Gleixner,
	the arch/x86 maintainers, Linux Kernel Mailing List,
	Borislav Petkov

On Thu, Feb 27, 2020 at 04:21:32PM +0100, Ard Biesheuvel wrote:
> On Thu, 27 Feb 2020 at 16:16, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > On Thu, Feb 27, 2020 at 09:12:29AM +0100, Ingo Molnar wrote:
> > >
> > > * Arvind Sankar <nivedita@alum.mit.edu> wrote:
> > >
> > > > Commit ef5a7b5eb13e ("efi/x86: Remove GDT setup from efi_main")
> > > > introduced GDT setup into the 32-bit kernel's startup_32, and reloads
> > > > the GDTR after relocating the kernel for paranoia's sake.
> > > >
> > > > Commit 32d009137a56 ("x86/boot: Reload GDTR after copying to the end of
> > > > the buffer") introduced a similar GDTR reload in the 64-bit kernel.
> > > >
> > > > The GDTR is adjusted by init_size - _end, however this may not be the
> > > > correct offset to apply if the kernel was loaded at a misaligned address
> > > > or below LOAD_PHYSICAL_ADDR, as in that case the decompression buffer
> > > > has an additional offset from the original load address.
> > > >
> > > > This should never happen for a conformant bootloader, but we're being
> > > > paranoid anyway, so just store the new GDT address in there instead of
> > > > adding any offsets, which is simpler as well.
> > > >
> > > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > Fixes: ef5a7b5eb13e ("efi/x86: Remove GDT setup from efi_main")
> > > > Fixes: 32d009137a56 ("x86/boot: Reload GDTR after copying to the end of the buffer")
> > >
> > > Have you or anyone else observed this condition practice, or have a
> > > suspicion that this could happen - or is this a mostly theoretical
> > > concern?
> > >
> > > Thanks,
> > >
> > >       Ingo
> >
> > Right now it's a theoretical concern.
> >
> > I'm working on another patch, to tell the EFI firmware PE loader what
> > the kernel's preferred address is, so that we can avoid having to
> > relocate the kernel in the EFI stub in most cases (ie if the PE loader
> > manages to load us at that address). With those changes, the required
> > adjustment won't be init_size - _end any more, and while fixing it up
> > there, I noticed that it could already be the case that the required
> > adjustment is different.
> >
> 
> Do you mean setting the image address in the PE/COFF header to the
> preferred address?

Yep. I'm doing that and then making a few adjustments to the PE entry
code and head_* so that it can decompress starting at image_base instead
of startup_32.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/1] x86/boot/compressed: Fix reloading of GDTR post-relocation
  2020-02-27 15:54             ` Arvind Sankar
@ 2020-02-27 17:47               ` Ard Biesheuvel
  2020-02-27 18:03                 ` Arvind Sankar
  0 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2020-02-27 17:47 UTC (permalink / raw)
  To: Arvind Sankar
  Cc: Ingo Molnar, linux-efi, Thomas Gleixner,
	the arch/x86 maintainers, Linux Kernel Mailing List,
	Borislav Petkov

On Thu, 27 Feb 2020 at 16:54, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Thu, Feb 27, 2020 at 04:21:32PM +0100, Ard Biesheuvel wrote:
> > On Thu, 27 Feb 2020 at 16:16, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> > >
> > > On Thu, Feb 27, 2020 at 09:12:29AM +0100, Ingo Molnar wrote:
> > > >
> > > > * Arvind Sankar <nivedita@alum.mit.edu> wrote:
> > > >
> > > > > Commit ef5a7b5eb13e ("efi/x86: Remove GDT setup from efi_main")
> > > > > introduced GDT setup into the 32-bit kernel's startup_32, and reloads
> > > > > the GDTR after relocating the kernel for paranoia's sake.
> > > > >
> > > > > Commit 32d009137a56 ("x86/boot: Reload GDTR after copying to the end of
> > > > > the buffer") introduced a similar GDTR reload in the 64-bit kernel.
> > > > >
> > > > > The GDTR is adjusted by init_size - _end, however this may not be the
> > > > > correct offset to apply if the kernel was loaded at a misaligned address
> > > > > or below LOAD_PHYSICAL_ADDR, as in that case the decompression buffer
> > > > > has an additional offset from the original load address.
> > > > >
> > > > > This should never happen for a conformant bootloader, but we're being
> > > > > paranoid anyway, so just store the new GDT address in there instead of
> > > > > adding any offsets, which is simpler as well.
> > > > >
> > > > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > > Fixes: ef5a7b5eb13e ("efi/x86: Remove GDT setup from efi_main")
> > > > > Fixes: 32d009137a56 ("x86/boot: Reload GDTR after copying to the end of the buffer")
> > > >
> > > > Have you or anyone else observed this condition practice, or have a
> > > > suspicion that this could happen - or is this a mostly theoretical
> > > > concern?
> > > >
> > > > Thanks,
> > > >
> > > >       Ingo
> > >
> > > Right now it's a theoretical concern.
> > >
> > > I'm working on another patch, to tell the EFI firmware PE loader what
> > > the kernel's preferred address is, so that we can avoid having to
> > > relocate the kernel in the EFI stub in most cases (ie if the PE loader
> > > manages to load us at that address). With those changes, the required
> > > adjustment won't be init_size - _end any more, and while fixing it up
> > > there, I noticed that it could already be the case that the required
> > > adjustment is different.
> > >
> >
> > Do you mean setting the image address in the PE/COFF header to the
> > preferred address?
>
> Yep. I'm doing that and then making a few adjustments to the PE entry
> code and head_* so that it can decompress starting at image_base instead
> of startup_32.

Interesting. I am going to rip most of the EFI handover protocol stuff
out of OVMF, since it is mostly unnecessary, and having the PE/COFF
loader put the image in the correct place right away is a nice
complimentary improvement to that. (Note that the OVMF implementation
of the EFI handover protocol does not currently honor the preferred
address from the setup header anyway)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/1] x86/boot/compressed: Fix reloading of GDTR post-relocation
  2020-02-27 17:47               ` Ard Biesheuvel
@ 2020-02-27 18:03                 ` Arvind Sankar
  2020-02-29  9:24                   ` Ingo Molnar
  0 siblings, 1 reply; 15+ messages in thread
From: Arvind Sankar @ 2020-02-27 18:03 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Arvind Sankar, Ingo Molnar, linux-efi, Thomas Gleixner,
	the arch/x86 maintainers, Linux Kernel Mailing List,
	Borislav Petkov

On Thu, Feb 27, 2020 at 06:47:55PM +0100, Ard Biesheuvel wrote:
> 
> Interesting. I am going to rip most of the EFI handover protocol stuff
> out of OVMF, since it is mostly unnecessary, and having the PE/COFF
> loader put the image in the correct place right away is a nice
> complimentary improvement to that. (Note that the OVMF implementation
> of the EFI handover protocol does not currently honor the preferred
> address from the setup header anyway)

Yeah, for my testing I'm running the image from the EFI shell, which
enters via PE entry point and honors the pref address.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/1] x86/boot/compressed: Fix reloading of GDTR post-relocation
  2020-02-27 18:03                 ` Arvind Sankar
@ 2020-02-29  9:24                   ` Ingo Molnar
  2020-02-29 16:50                     ` Arvind Sankar
  0 siblings, 1 reply; 15+ messages in thread
From: Ingo Molnar @ 2020-02-29  9:24 UTC (permalink / raw)
  To: Arvind Sankar
  Cc: Ard Biesheuvel, linux-efi, Thomas Gleixner,
	the arch/x86 maintainers, Linux Kernel Mailing List,
	Borislav Petkov


* Arvind Sankar <nivedita@alum.mit.edu> wrote:

> On Thu, Feb 27, 2020 at 06:47:55PM +0100, Ard Biesheuvel wrote:
> > 
> > Interesting. I am going to rip most of the EFI handover protocol stuff
> > out of OVMF, since it is mostly unnecessary, and having the PE/COFF
> > loader put the image in the correct place right away is a nice
> > complimentary improvement to that. (Note that the OVMF implementation
> > of the EFI handover protocol does not currently honor the preferred
> > address from the setup header anyway)
> 
> Yeah, for my testing I'm running the image from the EFI shell, which
> enters via PE entry point and honors the pref address.

So with KASLR, which is the distro default on most x86 distros, we'll 
relocate the kernel to another address anyway, right?

But telling the bootloader the preferred address would avoid any 
relocation overhead even in this case, right?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/1] x86/boot/compressed: Fix reloading of GDTR post-relocation
  2020-02-29  9:24                   ` Ingo Molnar
@ 2020-02-29 16:50                     ` Arvind Sankar
  0 siblings, 0 replies; 15+ messages in thread
From: Arvind Sankar @ 2020-02-29 16:50 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Arvind Sankar, Ard Biesheuvel, linux-efi, Thomas Gleixner,
	the arch/x86 maintainers, Linux Kernel Mailing List,
	Borislav Petkov

On Sat, Feb 29, 2020 at 10:24:25AM +0100, Ingo Molnar wrote:
> 
> * Arvind Sankar <nivedita@alum.mit.edu> wrote:
> 
> > On Thu, Feb 27, 2020 at 06:47:55PM +0100, Ard Biesheuvel wrote:
> > > 
> > > Interesting. I am going to rip most of the EFI handover protocol stuff
> > > out of OVMF, since it is mostly unnecessary, and having the PE/COFF
> > > loader put the image in the correct place right away is a nice
> > > complimentary improvement to that. (Note that the OVMF implementation
> > > of the EFI handover protocol does not currently honor the preferred
> > > address from the setup header anyway)
> > 
> > Yeah, for my testing I'm running the image from the EFI shell, which
> > enters via PE entry point and honors the pref address.
> 
> So with KASLR, which is the distro default on most x86 distros, we'll 
> relocate the kernel to another address anyway, right?
> 
> But telling the bootloader the preferred address would avoid any 
> relocation overhead even in this case, right?
> 
> Thanks,
> 
> 	Ingo

If I understand correctly, pref_address was added to try to avoid having
to apply the relocations (vmlinux.relocs) by loading at the link-time
address if possible, and you're saying that with KASLR, we have to do
relocations anyway; and without KASLR the bootloader knows the preferred
address from the setup header already?

The patch I'm working on is meant to address the case when we are loaded
via the EFI LoadImage service, which is a generic PE loader, and hence
doesn't look at the pref_address in the setup header. We currently will
end up with at least 2 copies of the kernel, with 1 additional copy if
KASLR is enabled. The loader puts us somewhere, the EFI stub then makes
a copy trying to move us to pref_address, and then KASLR will allocate
an additional copy (not really a copy, but a buffer of the same size to
decompress into). By telling the PE loader what the preferred address is
I'm trying to avoid the EFI stub creating an additional copy, so we'll
have 1 copy, or 2 with KASLR.

That was the original motivation for the patch. As I've been working on
it, I'm thinking that the copy in the EFI stub can be avoided in more
cases.

AFAICT, pref_address is completely irrelevant on 64-bit, which will have
relocations applied only if KASLR is enabled and changes the virtual
address, but it never cares what physical address it's running out of.
It only requires being above LOAD_PHYSICAL_ADDR and below 2^46 or 2^52.
All we need to do is align the decompression buffer correctly.

For 32-bit, loading at pref_address can avoid relocations being applied,
but if we're not already loaded there in the first place, trying to
allocate a new buffer and copying the whole image is probably not worth
it. It does have more restrictions on physical addresses in that it
can't run at too high a physical address, so it may still be necessary
to relocate to avoid that.

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-02-29 16:50 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-24 13:38 [GIT PULL v2] EFI updates for v5.7 Ard Biesheuvel
2020-02-26 14:27 ` Ingo Molnar
2020-02-26 14:50   ` Ard Biesheuvel
2020-02-26 20:45   ` [PATCH 0/1] Small fix to boot-time GDT handling Arvind Sankar
2020-02-26 23:00     ` [PATCH v2 " Arvind Sankar
2020-02-26 23:00     ` [PATCH v2 1/1] x86/boot/compressed: Fix reloading of GDTR post-relocation Arvind Sankar
2020-02-27  8:12       ` Ingo Molnar
2020-02-27 15:16         ` Arvind Sankar
2020-02-27 15:21           ` Ard Biesheuvel
2020-02-27 15:54             ` Arvind Sankar
2020-02-27 17:47               ` Ard Biesheuvel
2020-02-27 18:03                 ` Arvind Sankar
2020-02-29  9:24                   ` Ingo Molnar
2020-02-29 16:50                     ` Arvind Sankar
2020-02-26 20:45   ` [PATCH 1/1] x86/boot/compressed/32: " Arvind Sankar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).