[PATCH v3 0/6] efi: implement generic compressed boot support

[PATCH v3 0/6] efi: implement generic compressed boot support

[Ard Biesheuvel]

Relatively modern architectures such as arm64 or RISC-V don't implement
a self-decompressing kernel, and leave it up to the bootloader to
decompress the compressed image before executing it. For bare metal
boot, this policy makes sense, as a self-decompressing image essentially
duplicates a lot of fiddly preparation work to create a 1:1 mapping and
set up the C runtime, and to discover or infer where DRAM lives from
device trees or other firmware tables.

For EFI boot, the situation is a bit different: the EFI entrypoint is
called with a 1:1 cached mapping covering all of DRAM already active,
and with a stack, a heap, a memory map and boot services to load and
start images. This means it is rather trivial to implement a
self-decompressing wrapper for EFI boot in a generic manner, and reuse
it across architectures that implement EFI boot.

The only slight downside is that when UEFI secure boot is enabled, the
generic LoadImage/StartImage only allow signed images to be loaded and
started, and we would prefer to avoid the need to sign both the inner
and outer PE/COFF images.

However, the only truly generic and portable way to achieve this is to
rely on LoadImage/StartImage as the EFI spec defines them, and avoid
making assumptions about how things might work under the hood, and how
we might circumvent that. This includes just loading the image into
memory and jumping to the PE entry point: in the context of secure boot,
measured boot and other hardening measures the firmware may take (such
as disallowing mappings that are both writable and executable), using
the firmware's image loading API is the only maintainable choice.

For this reason, this version of the series includes support for signing
the images using sbsign, if the signing key and cert are specified in
Kconfig.

The code is wired up for arm64 and RISC-V. The latter was build tested
only. I also tested the code with the upcoming LoongArch EFI stub
support, which built correctly (using binutils 2.39) but needs some
additional work before it will run as expected.

Changes since v2:
- drop some of the refactoring work to make efi_printk() available in
  the decompressor, and just use fixed strings instead;
- provide memcpy/memmove/memset based on the UEFI boot services, instead
  of having to specify for each architecture how to wire these up;
- drop PI/DXE based signature check circumvention, and just sign the
  inner image instead, if needed;
- add a header to the zimage binary that identifies it as a EFI zboot
  image, and describes the compression algorithm and where the payload
  lives in the image - this might be used by non-EFI loaders to locate
  and decompress the bare metal image, given that the EFI zboot one is
  not a hybrid like the one it encapsulates.

Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Peter Jones <pjones@redhat.com>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Atish Patra <atishp@atishpatra.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Huacai Chen <chenhuacai@loongson.cn>
Cc: Lennart Poettering <lennart@poettering.net>
Cc: Jeremy Linton <jeremy.linton@arm.com>

Ard Biesheuvel (6):
  efi/libstub: use EFI provided memcpy/memset routines
  efi/libstub: add some missing boot service prototypes
  efi/libstub: move efi_system_table global var into separate object
  efi/libstub: implement generic EFI zboot
  arm64: efi: enable generic EFI compressed boot
  riscv: efi: enable generic EFI compressed boot

 arch/arm64/Makefile                         |   7 +-
 arch/arm64/boot/Makefile                    |   6 +
 arch/arm64/kernel/image-vars.h              |  13 --
 arch/riscv/Makefile                         |   5 +
 arch/riscv/boot/Makefile                    |   6 +
 arch/riscv/kernel/image-vars.h              |   9 --
 drivers/firmware/efi/Kconfig                |  31 ++++-
 drivers/firmware/efi/libstub/Makefile       |  11 +-
 drivers/firmware/efi/libstub/Makefile.zboot |  69 ++++++++++
 drivers/firmware/efi/libstub/efi-stub.c     |   2 -
 drivers/firmware/efi/libstub/efistub.h      |  12 +-
 drivers/firmware/efi/libstub/intrinsics.c   |  30 +++++
 drivers/firmware/efi/libstub/systable.c     |   8 ++
 drivers/firmware/efi/libstub/zboot-header.S | 139 ++++++++++++++++++++
 drivers/firmware/efi/libstub/zboot.c        | 101 ++++++++++++++
 drivers/firmware/efi/libstub/zboot.lds      |  39 ++++++
 16 files changed, 453 insertions(+), 35 deletions(-)
 create mode 100644 drivers/firmware/efi/libstub/Makefile.zboot
 create mode 100644 drivers/firmware/efi/libstub/intrinsics.c
 create mode 100644 drivers/firmware/efi/libstub/systable.c
 create mode 100644 drivers/firmware/efi/libstub/zboot-header.S
 create mode 100644 drivers/firmware/efi/libstub/zboot.c
 create mode 100644 drivers/firmware/efi/libstub/zboot.lds

-- 
2.35.1

[PATCH v3 1/6] efi/libstub: use EFI provided memcpy/memset routines

[Ard Biesheuvel]

The stub is used in different execution environments, but on arm64 and
RISC-V, we still use the core kernel's implementation of memcpy and
memset, as they are just a branch instruction away, and can generally be
reused even from code such as the EFI stub that runs in a completely
different address space.

KAsan complicates this slightly, resulting in the need for some hacks to
expose the uninstrumented, __ prefixed versions as the normal ones, as
the latter are instrumented to include the KAsan checks, which only work
in the core kernel.

Unfortunately, #define'ing memcpy to __memcpy when building C code does
not guarantee that no explicit memcpy() calls will be emitted. And with
the upcoming zboot support, which consists of a separate binary which
therefore needs its own implementation of memcpy/memset anyway, it's
better to provide one explicitly instead of linking to the existing one.

Given that EFI exposes implementations of memmove() and memset() via the
boot services table, let's wire those up in the appropriate way, and
drop the references to the core kernel ones.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/kernel/image-vars.h            | 13 ---------
 arch/riscv/kernel/image-vars.h            |  9 ------
 drivers/firmware/efi/libstub/Makefile     |  3 +-
 drivers/firmware/efi/libstub/efistub.h    |  4 +--
 drivers/firmware/efi/libstub/intrinsics.c | 30 ++++++++++++++++++++
 5 files changed, 34 insertions(+), 25 deletions(-)

diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index 3285b9847871..6e94c728295b 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -23,9 +23,6 @@ PROVIDE(__efistub_primary_entry		= primary_entry);
  */
 PROVIDE(__efistub_memcmp		= __pi_memcmp);
 PROVIDE(__efistub_memchr		= __pi_memchr);
-PROVIDE(__efistub_memcpy		= __pi_memcpy);
-PROVIDE(__efistub_memmove		= __pi_memmove);
-PROVIDE(__efistub_memset		= __pi_memset);
 PROVIDE(__efistub_strlen		= __pi_strlen);
 PROVIDE(__efistub_strnlen		= __pi_strnlen);
 PROVIDE(__efistub_strcmp		= __pi_strcmp);
@@ -38,16 +35,6 @@ PROVIDE(__efistub__edata		= _edata);
 PROVIDE(__efistub_screen_info		= screen_info);
 PROVIDE(__efistub__ctype		= _ctype);
 
-/*
- * The __ prefixed memcpy/memset/memmove symbols are provided by KASAN, which
- * instruments the conventional ones. Therefore, any references from the EFI
- * stub or other position independent, low level C code should be redirected to
- * the non-instrumented versions as well.
- */
-PROVIDE(__efistub___memcpy		= __pi_memcpy);
-PROVIDE(__efistub___memmove		= __pi_memmove);
-PROVIDE(__efistub___memset		= __pi_memset);
-
 PROVIDE(__pi___memcpy			= __pi_memcpy);
 PROVIDE(__pi___memmove			= __pi_memmove);
 PROVIDE(__pi___memset			= __pi_memset);
diff --git a/arch/riscv/kernel/image-vars.h b/arch/riscv/kernel/image-vars.h
index 71a76a623257..d6e5f739905e 100644
--- a/arch/riscv/kernel/image-vars.h
+++ b/arch/riscv/kernel/image-vars.h
@@ -25,21 +25,12 @@
  */
 __efistub_memcmp		= memcmp;
 __efistub_memchr		= memchr;
-__efistub_memcpy		= memcpy;
-__efistub_memmove		= memmove;
-__efistub_memset		= memset;
 __efistub_strlen		= strlen;
 __efistub_strnlen		= strnlen;
 __efistub_strcmp		= strcmp;
 __efistub_strncmp		= strncmp;
 __efistub_strrchr		= strrchr;
 
-#ifdef CONFIG_KASAN
-__efistub___memcpy		= memcpy;
-__efistub___memmove		= memmove;
-__efistub___memset		= memset;
-#endif
-
 __efistub__start		= _start;
 __efistub__start_kernel		= _start_kernel;
 __efistub__end			= _end;
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index d0537573501e..d7303c94b4a7 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -55,7 +55,8 @@ KCOV_INSTRUMENT			:= n
 lib-y				:= efi-stub-helper.o gop.o secureboot.o tpm.o \
 				   file.o mem.o random.o randomalloc.o pci.o \
 				   skip_spaces.o lib-cmdline.o lib-ctype.o \
-				   alignedmem.o relocate.o vsprintf.o
+				   alignedmem.o relocate.o vsprintf.o \
+				   intrinsics.o
 
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 efi-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index ab9e990447d3..a3377f0b1251 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -280,8 +280,8 @@ union efi_boot_services {
 		void *install_multiple_protocol_interfaces;
 		void *uninstall_multiple_protocol_interfaces;
 		void *calculate_crc32;
-		void *copy_mem;
-		void *set_mem;
+		void (__efiapi *copy_mem)(void *, const void *, unsigned long);
+		void (__efiapi *set_mem)(void *, unsigned long, unsigned char);
 		void *create_event_ex;
 	};
 	struct {
diff --git a/drivers/firmware/efi/libstub/intrinsics.c b/drivers/firmware/efi/libstub/intrinsics.c
new file mode 100644
index 000000000000..a04ab39292b6
--- /dev/null
+++ b/drivers/firmware/efi/libstub/intrinsics.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/efi.h>
+#include <asm/efi.h>
+#include <asm/string.h>
+
+#include "efistub.h"
+
+#ifdef CONFIG_KASAN
+#undef memcpy
+#undef memmove
+#undef memset
+void *__memcpy(void *__dest, const void *__src, size_t __n) __alias(memcpy);
+void *__memmove(void *__dest, const void *__src, size_t count) __alias(memmove);
+void *__memset(void *s, int c, size_t count) __alias(memset);
+#endif
+
+void *memcpy(void *dst, const void *src, size_t len)
+{
+	efi_bs_call(copy_mem, dst, src, len);
+	return dst;
+}
+
+extern void *memmove(void *dst, const void *src, size_t len) __alias(memcpy);
+
+void *memset(void *dst, int c, size_t len)
+{
+	efi_bs_call(set_mem, dst, len, c & U8_MAX);
+	return dst;
+}
-- 
2.35.1

[PATCH v3 2/6] efi/libstub: add some missing boot service prototypes

[Ard Biesheuvel]

Define the correct prototypes for the load_image/start_image boot
service pointers so we can call them from the EFI zboot code.

Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/efistub.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index a3377f0b1251..fbf2c83d6bf8 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -254,8 +254,12 @@ union efi_boot_services {
 							    efi_handle_t *);
 		efi_status_t (__efiapi *install_configuration_table)(efi_guid_t *,
 								     void *);
-		void *load_image;
-		void *start_image;
+		efi_status_t (__efiapi *load_image)(bool, efi_handle_t,
+						    efi_device_path_protocol_t *,
+						    void *, unsigned long,
+						    efi_handle_t *);
+		efi_status_t (__efiapi *start_image)(efi_handle_t, unsigned long *,
+						     efi_char16_t **);
 		efi_status_t __noreturn (__efiapi *exit)(efi_handle_t,
 							 efi_status_t,
 							 unsigned long,
-- 
2.35.1

[PATCH v3 3/6] efi/libstub: move efi_system_table global var into separate object

[Ard Biesheuvel]

To avoid pulling in the wrong object when using the libstub static
library to build the decompressor, define efi_system_table in a separate
compilation unit.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/Makefile   | 2 +-
 drivers/firmware/efi/libstub/efi-stub.c | 2 --
 drivers/firmware/efi/libstub/systable.c | 8 ++++++++
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index d7303c94b4a7..1406dc78edaa 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -56,7 +56,7 @@ lib-y				:= efi-stub-helper.o gop.o secureboot.o tpm.o \
 				   file.o mem.o random.o randomalloc.o pci.o \
 				   skip_spaces.o lib-cmdline.o lib-ctype.o \
 				   alignedmem.o relocate.o vsprintf.o \
-				   intrinsics.o
+				   intrinsics.o systable.o
 
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 efi-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index f515394cce6e..ad179632f99f 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -49,8 +49,6 @@
 static u64 virtmap_base = EFI_RT_VIRTUAL_BASE;
 static bool flat_va_mapping;
 
-const efi_system_table_t *efi_system_table;
-
 static struct screen_info *setup_graphics(void)
 {
 	efi_guid_t gop_proto = EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID;
diff --git a/drivers/firmware/efi/libstub/systable.c b/drivers/firmware/efi/libstub/systable.c
new file mode 100644
index 000000000000..91d016b02f8c
--- /dev/null
+++ b/drivers/firmware/efi/libstub/systable.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/efi.h>
+#include <asm/efi.h>
+
+#include "efistub.h"
+
+const efi_system_table_t *efi_system_table;
-- 
2.35.1

[PATCH v3 4/6] efi/libstub: implement generic EFI zboot

[Ard Biesheuvel]

Implement a minimal EFI app that decompresses the real kernel image and
launches it using the firmware's LoadImage and StartImage boot services.
This removes the need for any arch-specific hacks.

Note that on systems that have UEFI secure boot policies enabled,
LoadImage/StartImage require images to be signed, or their hashes known
a priori, in order to be permitted to boot.

There are various possible strategies to work around this requirement,
but they all rely either on overriding internal PI/DXE protocols (which
are not part of the EFI spec) or omitting the firmware provided
LoadImage() and StartImage() boot services, which is also undesirable,
given that they encapsulate platform specific policies related to secure
boot and measured boot, but also related to memory permissions (whether
or not and which types of heap allocations have both write and execute
permissions.)

The only generic and truly portable way around this is to simply sign
both the inner and the outer image with the same key/cert pair, so this
is what is implemented here.

BZIP2 has been omitted from the set of supported compression algorithms,
given that its performance is mediocre both in speed and size, and it
uses a disproportionate amount of memory. For optimal compression, use
LZMA. For the fastest boot speed, use LZO.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/Kconfig                |  31 ++++-
 drivers/firmware/efi/libstub/Makefile       |   8 +-
 drivers/firmware/efi/libstub/Makefile.zboot |  69 ++++++++++
 drivers/firmware/efi/libstub/zboot-header.S | 139 ++++++++++++++++++++
 drivers/firmware/efi/libstub/zboot.c        | 101 ++++++++++++++
 drivers/firmware/efi/libstub/zboot.lds      |  39 ++++++
 6 files changed, 382 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 6cb7384ad2ac..0c7beb8e3633 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -105,9 +105,36 @@ config EFI_RUNTIME_WRAPPERS
 config EFI_GENERIC_STUB
 	bool
 
+config EFI_ZBOOT
+	bool "Enable the generic EFI decompressor"
+	depends on EFI_GENERIC_STUB && !ARM
+	select HAVE_KERNEL_GZIP
+	select HAVE_KERNEL_LZ4
+	select HAVE_KERNEL_LZMA
+	select HAVE_KERNEL_LZO
+
+config EFI_ZBOOT_SIGNED
+	bool "Sign the EFI decompressor for UEFI secure boot"
+	depends on EFI_ZBOOT
+	help
+	  Use the 'sbsign' command line tool (which must exist on the host
+	  path) to sign both the EFI decompressor PE/COFF image, as well as the
+	  encapsulated PE/COFF image, which is subsequently compressed and
+	  wrapped by the former image.
+
+config EFI_ZBOOT_SIGNING_CERT
+	string "Certificate to use for signing the compressed EFI boot image"
+	depends on EFI_ZBOOT_SIGNED
+	default ""
+
+config EFI_ZBOOT_SIGNING_KEY
+	string "Private key to use for signing the compressed EFI boot image"
+	depends on EFI_ZBOOT_SIGNED
+	default ""
+
 config EFI_ARMSTUB_DTB_LOADER
 	bool "Enable the DTB loader"
-	depends on EFI_GENERIC_STUB && !RISCV
+	depends on EFI_GENERIC_STUB && !RISCV && !EFI_ZBOOT
 	default y
 	help
 	  Select this config option to add support for the dtb= command
@@ -124,7 +151,7 @@ config EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER
 	bool "Enable the command line initrd loader" if !X86
 	depends on EFI_STUB && (EFI_GENERIC_STUB || X86)
 	default y if X86
-	depends on !RISCV
+	depends on !RISCV && !EFI_ZBOOT
 	help
 	  Select this config option to add support for the initrd= command
 	  line parameter, allowing an initrd that resides on the same volume
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 1406dc78edaa..a3d3d38d5afd 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -73,6 +73,11 @@ lib-$(CONFIG_X86)		+= x86-stub.o
 lib-$(CONFIG_RISCV)		+= riscv-stub.o
 CFLAGS_arm32-stub.o		:= -DTEXT_OFFSET=$(TEXT_OFFSET)
 
+lib-$(CONFIG_EFI_ZBOOT)		+= zboot.o
+
+extra-y				:= $(lib-y)
+lib-y				:= $(patsubst %.o,%.stub.o,$(lib-y))
+
 # Even when -mbranch-protection=none is set, Clang will generate a
 # .note.gnu.property for code-less object files (like lib/ctype.c),
 # so work around this by explicitly removing the unwanted section.
@@ -112,9 +117,6 @@ STUBCOPY_RELOC-$(CONFIG_ARM)	:= R_ARM_ABS
 # a verification pass to see if any absolute relocations exist in any of the
 # object files.
 #
-extra-y				:= $(lib-y)
-lib-y				:= $(patsubst %.o,%.stub.o,$(lib-y))
-
 STUBCOPY_FLAGS-$(CONFIG_ARM64)	+= --prefix-alloc-sections=.init \
 				   --prefix-symbols=__efistub_
 STUBCOPY_RELOC-$(CONFIG_ARM64)	:= R_AARCH64_ABS
diff --git a/drivers/firmware/efi/libstub/Makefile.zboot b/drivers/firmware/efi/libstub/Makefile.zboot
new file mode 100644
index 000000000000..38dee29103ae
--- /dev/null
+++ b/drivers/firmware/efi/libstub/Makefile.zboot
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: GPL-2.0
+
+# to be include'd by arch/$(ARCH)/boot/Makefile after setting
+# EFI_ZBOOT_PAYLOAD, EFI_ZBOOT_BFD_TARGET and EFI_ZBOOT_MACH_TYPE
+
+comp-type-$(CONFIG_KERNEL_GZIP)		:= gzip
+comp-type-$(CONFIG_KERNEL_LZ4)		:= lz4
+comp-type-$(CONFIG_KERNEL_LZMA)		:= lzma
+comp-type-$(CONFIG_KERNEL_LZO)		:= lzo
+
+# in GZIP, the appended le32 carrying the uncompressed size is part of the
+# format, but in other cases, we just append it at the end for convenience,
+# causing the original tools to complain when checking image integrity.
+# So disregard it when calculating the payload size in the zimage header.
+zimage-method-y				:= $(comp-type-y)_with_size
+zimage-size-len-y			:= 4
+
+zimage-method-$(CONFIG_KERNEL_GZIP)	:= gzip
+zimage-size-len-$(CONFIG_KERNEL_GZIP)	:= 0
+
+quiet_cmd_sbsign = SBSIGN  $@
+      cmd_sbsign = sbsign --out $@ $< \
+		   --key $(CONFIG_EFI_ZBOOT_SIGNING_KEY) \
+		   --cert $(CONFIG_EFI_ZBOOT_SIGNING_CERT) \
+		   2>/dev/null
+
+$(obj)/Image.signed: $(EFI_ZBOOT_PAYLOAD) FORCE
+	$(call if_changed,sbsign)
+
+ZBOOT_PAYLOAD-y				 := $(EFI_ZBOOT_PAYLOAD)
+ZBOOT_PAYLOAD-$(CONFIG_EFI_ZBOOT_SIGNED) := $(obj)/Image.signed
+
+$(obj)/zImage: $(ZBOOT_PAYLOAD-y) FORCE
+	$(call if_changed,$(zimage-method-y))
+
+OBJCOPYFLAGS_zImage.o := -I binary -O $(EFI_ZBOOT_BFD_TARGET) \
+			 --rename-section .data=.gzdata,load,alloc,readonly,contents
+$(obj)/zImage.o: $(obj)/zImage FORCE
+	$(call if_changed,objcopy)
+
+AFLAGS_zboot-header.o += -DMACHINE_TYPE=IMAGE_FILE_MACHINE_$(EFI_ZBOOT_MACH_TYPE) \
+			 -DZIMG_EFI_PATH="\"$(realpath $(obj)/zImage.efi.elf)\"" \
+			 -DZIMG_SIZE_LEN=$(zimage-size-len-y) \
+			 -DCOMP_TYPE="\"$(comp-type-y)\""
+
+$(obj)/zboot-header.o: $(srctree)/drivers/firmware/efi/libstub/zboot-header.S FORCE
+	$(call if_changed_rule,as_o_S)
+
+ZBOOT_DEPS := $(obj)/zboot-header.o $(objtree)/drivers/firmware/efi/libstub/lib.a
+
+LDFLAGS_zImage.efi.elf := -T $(srctree)/drivers/firmware/efi/libstub/zboot.lds
+$(obj)/zImage.efi.elf: $(obj)/zImage.o $(ZBOOT_DEPS) FORCE
+	$(call if_changed,ld)
+
+ZIMAGE_EFI-y				:= zImage.efi
+ZIMAGE_EFI-$(CONFIG_EFI_ZBOOT_SIGNED)	:= zImage.efi.unsigned
+
+OBJCOPYFLAGS_$(ZIMAGE_EFI-y) := -O binary
+$(obj)/$(ZIMAGE_EFI-y): $(obj)/zImage.efi.elf FORCE
+	$(call if_changed,objcopy)
+
+targets += zboot-header.o zImage zImage.o zImage.efi.elf zImage.efi
+
+ifneq ($(CONFIG_EFI_ZBOOT_SIGNED),)
+$(obj)/zImage.efi: $(obj)/zImage.efi.unsigned FORCE
+	$(call if_changed,sbsign)
+
+targets += Image.signed zImage.efi.unsigned
+endif
diff --git a/drivers/firmware/efi/libstub/zboot-header.S b/drivers/firmware/efi/libstub/zboot-header.S
new file mode 100644
index 000000000000..ee6a3cd69773
--- /dev/null
+++ b/drivers/firmware/efi/libstub/zboot-header.S
@@ -0,0 +1,139 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <linux/pe.h>
+
+#ifdef CONFIG_64BIT
+	.set		.Lextra_characteristics, 0x0
+	.set		.Lpe_opt_magic, PE_OPT_MAGIC_PE32PLUS
+#else
+	.set		.Lextra_characteristics, IMAGE_FILE_32BIT_MACHINE
+	.set		.Lpe_opt_magic, PE_OPT_MAGIC_PE32
+#endif
+
+	.section	".head", "a"
+	.globl		__efistub_efi_zboot_header
+__efistub_efi_zboot_header:
+.Ldoshdr:
+	.long		MZ_MAGIC
+	.ascii		"zimg"					// image type
+	.long		__efistub__gzdata_start - .Ldoshdr	// payload offset
+	.long		__efistub__gzdata_size - ZIMG_SIZE_LEN	// payload size
+	.long		0, 0					// reserved
+	.asciz		COMP_TYPE				// compression type
+	.org		.Ldoshdr + 0x3c
+	.long		.Lpehdr - .Ldoshdr			// PE header offset
+
+.Lpehdr:
+	.long		PE_MAGIC
+	.short		MACHINE_TYPE
+	.short		.Lsection_count
+	.long		0
+	.long		0
+	.long		0
+	.short		.Lsection_table - .Loptional_header
+	.short		IMAGE_FILE_DEBUG_STRIPPED | \
+			IMAGE_FILE_EXECUTABLE_IMAGE | \
+			IMAGE_FILE_LINE_NUMS_STRIPPED |\
+			.Lextra_characteristics
+
+.Loptional_header:
+	.short		.Lpe_opt_magic
+	.byte		0, 0
+	.long		_etext - .Lefi_header_end
+	.long		__data_size
+	.long		0
+	.long		__efistub_efi_zboot_entry - .Ldoshdr
+	.long		.Lefi_header_end - .Ldoshdr
+
+#ifdef CONFIG_64BIT
+	.quad		0
+#else
+	.long		_etext - .Ldoshdr, 0x0
+#endif
+	.long		4096
+	.long		512
+	.short		0, 0
+	.short		LINUX_EFISTUB_MAJOR_VERSION	// MajorImageVersion
+	.short		LINUX_EFISTUB_MINOR_VERSION	// MinorImageVersion
+	.short		0, 0
+	.long		0
+	.long		_end - .Ldoshdr
+
+	.long		.Lefi_header_end - .Ldoshdr
+	.long		0
+	.short		IMAGE_SUBSYSTEM_EFI_APPLICATION
+	.short		0
+	.quad		0, 0, 0, 0
+	.long		0
+	.long		(.Lsection_table - .) / 8
+
+	.quad		0				// ExportTable
+	.quad		0				// ImportTable
+	.quad		0				// ResourceTable
+	.quad		0				// ExceptionTable
+	.quad		0				// CertificationTable
+	.quad		0				// BaseRelocationTable
+#ifdef CONFIG_DEBUG_EFI
+	.long		.Lefi_debug_table - .Ldoshdr	// DebugTable
+	.long		.Lefi_debug_table_size
+#endif
+
+.Lsection_table:
+	.ascii		".text\0\0\0"
+	.long		_etext - .Lefi_header_end
+	.long		.Lefi_header_end - .Ldoshdr
+	.long		_etext - .Lefi_header_end
+	.long		.Lefi_header_end - .Ldoshdr
+
+	.long		0, 0
+	.short		0, 0
+	.long		IMAGE_SCN_CNT_CODE | \
+			IMAGE_SCN_MEM_READ | \
+			IMAGE_SCN_MEM_EXECUTE
+
+	.ascii		".data\0\0\0"
+	.long		__data_size
+	.long		_etext - .Ldoshdr
+	.long		__data_rawsize
+	.long		_etext - .Ldoshdr
+
+	.long		0, 0
+	.short		0, 0
+	.long		IMAGE_SCN_CNT_INITIALIZED_DATA | \
+			IMAGE_SCN_MEM_READ | \
+			IMAGE_SCN_MEM_WRITE
+
+	.set		.Lsection_count, (. - .Lsection_table) / 40
+
+#ifdef CONFIG_DEBUG_EFI
+	.section	".rodata", "a"
+	.align		2
+.Lefi_debug_table:
+	// EFI_IMAGE_DEBUG_DIRECTORY_ENTRY
+	.long		0				// Characteristics
+	.long		0				// TimeDateStamp
+	.short		0				// MajorVersion
+	.short		0				// MinorVersion
+	.long		IMAGE_DEBUG_TYPE_CODEVIEW	// Type
+	.long		.Lefi_debug_entry_size		// SizeOfData
+	.long		0				// RVA
+	.long		.Lefi_debug_entry - .Ldoshdr	// FileOffset
+
+	.set		.Lefi_debug_table_size, . - .Lefi_debug_table
+	.previous
+
+.Lefi_debug_entry:
+	// EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY
+	.ascii		"NB10"				// Signature
+	.long		0				// Unknown
+	.long		0				// Unknown2
+	.long		0				// Unknown3
+
+	.asciz		ZIMG_EFI_PATH
+
+	.set		.Lefi_debug_entry_size, . - .Lefi_debug_entry
+#endif
+
+	.p2align	12
+.Lefi_header_end:
+
diff --git a/drivers/firmware/efi/libstub/zboot.c b/drivers/firmware/efi/libstub/zboot.c
new file mode 100644
index 000000000000..9cf968e90775
--- /dev/null
+++ b/drivers/firmware/efi/libstub/zboot.c
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/efi.h>
+#include <linux/pe.h>
+#include <asm/efi.h>
+
+#include "efistub.h"
+
+static unsigned char zboot_heap[SZ_64K] __aligned(64);
+static unsigned long free_mem_ptr, free_mem_end_ptr;
+
+#define STATIC static
+#if defined(CONFIG_KERNEL_GZIP)
+#include "../../../../lib/decompress_inflate.c"
+#elif defined(CONFIG_KERNEL_LZ4)
+#include "../../../../lib/decompress_unlz4.c"
+#elif defined(CONFIG_KERNEL_LZMA)
+#include "../../../../lib/decompress_unlzma.c"
+#elif defined(CONFIG_KERNEL_LZO)
+#include "../../../../lib/decompress_unlzo.c"
+#endif
+
+extern char _gzdata_start[], _gzdata_end[];
+extern u32 uncompressed_size __aligned(1);
+
+static void log(efi_char16_t str[])
+{
+	efi_call_proto(efi_table_attr(efi_system_table, con_out),
+		       output_string, L"EFI decompressor: ");
+	efi_call_proto(efi_table_attr(efi_system_table, con_out),
+		       output_string, str);
+}
+
+static void error(char *x)
+{
+	log(L"error() called from decompressor library\n");
+}
+
+efi_status_t __efiapi efi_zboot_entry(efi_handle_t handle,
+				      efi_system_table_t *systab)
+{
+	static efi_guid_t loaded_image = LOADED_IMAGE_PROTOCOL_GUID;
+	efi_loaded_image_t *parent, *child;
+	unsigned long image_buffer;
+	efi_handle_t child_handle;
+	efi_status_t status;
+	int ret;
+
+	WRITE_ONCE(efi_system_table, systab);
+
+	free_mem_ptr = (unsigned long)&zboot_heap;
+	free_mem_end_ptr = free_mem_ptr + sizeof(zboot_heap);
+
+	status = efi_bs_call(handle_protocol, handle, &loaded_image,
+			     (void **)&parent);
+	if (status != EFI_SUCCESS) {
+		log(L"Failed to locate parent's loaded image protocol\n");
+		return status;
+	}
+
+	status = efi_allocate_pages(uncompressed_size, &image_buffer, ULONG_MAX);
+	if (status != EFI_SUCCESS) {
+		log(L"Failed to allocate memory\n");
+		return status;
+	}
+
+	ret = __decompress(_gzdata_start, _gzdata_end - _gzdata_start, NULL,
+			   NULL, (unsigned char *)image_buffer, 0, NULL,
+			   error);
+	if (ret	< 0) {
+		log(L"Decompression failed\n");
+		return EFI_LOAD_ERROR;
+	}
+
+	status = efi_bs_call(load_image, false, handle, NULL,
+			     (void *)image_buffer, uncompressed_size,
+			     &child_handle);
+	if (status != EFI_SUCCESS) {
+		log(L"Failed to load image\n");
+		return status;
+	}
+
+	status = efi_bs_call(handle_protocol, child_handle, &loaded_image,
+			     (void **)&child);
+	if (status != EFI_SUCCESS) {
+		log(L"Failed to locate child's loaded image protocol\n");
+		return status;
+	}
+
+	// Copy the kernel command line
+	child->load_options = parent->load_options;
+	child->load_options_size = parent->load_options_size;
+
+	status = efi_bs_call(start_image, child_handle, NULL, NULL);
+	if (status != EFI_SUCCESS) {
+		log(L"StartImage() returned with error\n");
+		return status;
+	}
+
+	return EFI_SUCCESS;
+}
diff --git a/drivers/firmware/efi/libstub/zboot.lds b/drivers/firmware/efi/libstub/zboot.lds
new file mode 100644
index 000000000000..d6ba89a0c294
--- /dev/null
+++ b/drivers/firmware/efi/libstub/zboot.lds
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+ENTRY(__efistub_efi_zboot_header);
+
+SECTIONS
+{
+	.text : ALIGN(4096) {
+		*(.head)
+		*(.text* .init.text*)
+	}
+
+	.rodata : ALIGN(8) {
+		__efistub__gzdata_start = .;
+		*(.gzdata)
+		__efistub__gzdata_end = .;
+		*(.rodata* .init.rodata* .srodata*)
+		_etext = ALIGN(4096);
+		. = _etext;
+	}
+
+	.data : ALIGN(4096) {
+		*(.data* .init.data*)
+		_edata = ALIGN(512);
+		. = _edata;
+	}
+
+	.bss : {
+		*(.bss* .init.bss*)
+		_end = ALIGN(512);
+		. = _end;
+	}
+}
+
+PROVIDE(__efistub__gzdata_size = ABSOLUTE(. - __efistub__gzdata_start));
+
+PROVIDE(__efistub_uncompressed_size = __efistub__gzdata_end - 4);
+
+PROVIDE(__data_rawsize = ABSOLUTE(_edata - _etext));
+PROVIDE(__data_size = ABSOLUTE(_end - _etext));
-- 
2.35.1

Re: [PATCH v3 4/6] efi/libstub: implement generic EFI zboot

[Arnd Bergmann]

On Wed, Aug 17, 2022 at 1:03 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> BZIP2 has been omitted from the set of supported compression algorithms,
> given that its performance is mediocre both in speed and size, and it
> uses a disproportionate amount of memory. For optimal compression, use
> LZMA. For the fastest boot speed, use LZO.
...
> +config EFI_ZBOOT
> +       bool "Enable the generic EFI decompressor"
> +       depends on EFI_GENERIC_STUB && !ARM
> +       select HAVE_KERNEL_GZIP
> +       select HAVE_KERNEL_LZ4
> +       select HAVE_KERNEL_LZMA
> +       select HAVE_KERNEL_LZO

I hope I don't turn this into a bike-shed discussion, but it feels
like if you give
the choice between these four, you should also offer ZSTD, which combines
high compression ratio with fast decompression speed.
XZ is probably more widely installed than LZMA.

I would be happy with just gzip (to minimize build dependencies) and
zstd, but there is little harm in also including the other ones you have
here, or all seven of them.

        Arnd

Re: [PATCH v3 4/6] efi/libstub: implement generic EFI zboot

[Ard Biesheuvel]

On Wed, 17 Aug 2022 at 13:53, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Wed, Aug 17, 2022 at 1:03 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > BZIP2 has been omitted from the set of supported compression algorithms,
> > given that its performance is mediocre both in speed and size, and it
> > uses a disproportionate amount of memory. For optimal compression, use
> > LZMA. For the fastest boot speed, use LZO.
> ...
> > +config EFI_ZBOOT
> > +       bool "Enable the generic EFI decompressor"
> > +       depends on EFI_GENERIC_STUB && !ARM
> > +       select HAVE_KERNEL_GZIP
> > +       select HAVE_KERNEL_LZ4
> > +       select HAVE_KERNEL_LZMA
> > +       select HAVE_KERNEL_LZO
>
> I hope I don't turn this into a bike-shed discussion, but it feels
> like if you give
> the choice between these four, you should also offer ZSTD, which combines
> high compression ratio with fast decompression speed.
> XZ is probably more widely installed than LZMA.
>
> I would be happy with just gzip (to minimize build dependencies) and
> zstd, but there is little harm in also including the other ones you have
> here, or all seven of them.
>

Let's add whatever people feel is useful. The reason I dropped bzip2
is because it uses much more memory. This is not a problem per se, but
it seemed pointless to accommodate bzip2 if we have better options
anyway.

zstd seems trivial to add if i bump the heap size to 256k, and xz just
needs a little tweak so it doesn't redefine memmove and memcpy, but
beyond that, it's just copy/paste'ing the pattern another couple of
times.

Re: [PATCH v3 4/6] efi/libstub: implement generic EFI zboot

[Heinrich Schuchardt]

On 8/17/22 13:03, Ard Biesheuvel wrote:
> Implement a minimal EFI app that decompresses the real kernel image and
> launches it using the firmware's LoadImage and StartImage boot services.
> This removes the need for any arch-specific hacks.
> 
> Note that on systems that have UEFI secure boot policies enabled,
> LoadImage/StartImage require images to be signed, or their hashes known
> a priori, in order to be permitted to boot.
> 
> There are various possible strategies to work around this requirement,
> but they all rely either on overriding internal PI/DXE protocols (which
> are not part of the EFI spec) or omitting the firmware provided
> LoadImage() and StartImage() boot services, which is also undesirable,
> given that they encapsulate platform specific policies related to secure
> boot and measured boot, but also related to memory permissions (whether
> or not and which types of heap allocations have both write and execute
> permissions.)
> 
> The only generic and truly portable way around this is to simply sign
> both the inner and the outer image with the same key/cert pair, so this
> is what is implemented here.
> 
> BZIP2 has been omitted from the set of supported compression algorithms,
> given that its performance is mediocre both in speed and size, and it
> uses a disproportionate amount of memory. For optimal compression, use
> LZMA. For the fastest boot speed, use LZO.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>   drivers/firmware/efi/Kconfig                |  31 ++++-
>   drivers/firmware/efi/libstub/Makefile       |   8 +-
>   drivers/firmware/efi/libstub/Makefile.zboot |  69 ++++++++++
>   drivers/firmware/efi/libstub/zboot-header.S | 139 ++++++++++++++++++++
>   drivers/firmware/efi/libstub/zboot.c        | 101 ++++++++++++++
>   drivers/firmware/efi/libstub/zboot.lds      |  39 ++++++
>   6 files changed, 382 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
> index 6cb7384ad2ac..0c7beb8e3633 100644
> --- a/drivers/firmware/efi/Kconfig
> +++ b/drivers/firmware/efi/Kconfig
> @@ -105,9 +105,36 @@ config EFI_RUNTIME_WRAPPERS
>   config EFI_GENERIC_STUB
>   	bool
>   
> +config EFI_ZBOOT
> +	bool "Enable the generic EFI decompressor"
> +	depends on EFI_GENERIC_STUB && !ARM
> +	select HAVE_KERNEL_GZIP
> +	select HAVE_KERNEL_LZ4
> +	select HAVE_KERNEL_LZMA
> +	select HAVE_KERNEL_LZO
> +
> +config EFI_ZBOOT_SIGNED
> +	bool "Sign the EFI decompressor for UEFI secure boot"
> +	depends on EFI_ZBOOT
> +	help
> +	  Use the 'sbsign' command line tool (which must exist on the host
> +	  path) to sign both the EFI decompressor PE/COFF image, as well as the
> +	  encapsulated PE/COFF image, which is subsequently compressed and
> +	  wrapped by the former image.
> +
> +config EFI_ZBOOT_SIGNING_CERT
> +	string "Certificate to use for signing the compressed EFI boot image"
> +	depends on EFI_ZBOOT_SIGNED
> +	default ""
> +
> +config EFI_ZBOOT_SIGNING_KEY
> +	string "Private key to use for signing the compressed EFI boot image"
> +	depends on EFI_ZBOOT_SIGNED
> +	default ""
> +
>   config EFI_ARMSTUB_DTB_LOADER
>   	bool "Enable the DTB loader"
> -	depends on EFI_GENERIC_STUB && !RISCV
> +	depends on EFI_GENERIC_STUB && !RISCV && !EFI_ZBOOT
>   	default y
>   	help
>   	  Select this config option to add support for the dtb= command
> @@ -124,7 +151,7 @@ config EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER
>   	bool "Enable the command line initrd loader" if !X86
>   	depends on EFI_STUB && (EFI_GENERIC_STUB || X86)
>   	default y if X86
> -	depends on !RISCV
> +	depends on !RISCV && !EFI_ZBOOT
>   	help
>   	  Select this config option to add support for the initrd= command
>   	  line parameter, allowing an initrd that resides on the same volume
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index 1406dc78edaa..a3d3d38d5afd 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -73,6 +73,11 @@ lib-$(CONFIG_X86)		+= x86-stub.o
>   lib-$(CONFIG_RISCV)		+= riscv-stub.o
>   CFLAGS_arm32-stub.o		:= -DTEXT_OFFSET=$(TEXT_OFFSET)
>   
> +lib-$(CONFIG_EFI_ZBOOT)		+= zboot.o
> +
> +extra-y				:= $(lib-y)
> +lib-y				:= $(patsubst %.o,%.stub.o,$(lib-y))
> +
>   # Even when -mbranch-protection=none is set, Clang will generate a
>   # .note.gnu.property for code-less object files (like lib/ctype.c),
>   # so work around this by explicitly removing the unwanted section.
> @@ -112,9 +117,6 @@ STUBCOPY_RELOC-$(CONFIG_ARM)	:= R_ARM_ABS
>   # a verification pass to see if any absolute relocations exist in any of the
>   # object files.
>   #
> -extra-y				:= $(lib-y)
> -lib-y				:= $(patsubst %.o,%.stub.o,$(lib-y))
> -
>   STUBCOPY_FLAGS-$(CONFIG_ARM64)	+= --prefix-alloc-sections=.init \
>   				   --prefix-symbols=__efistub_
>   STUBCOPY_RELOC-$(CONFIG_ARM64)	:= R_AARCH64_ABS
> diff --git a/drivers/firmware/efi/libstub/Makefile.zboot b/drivers/firmware/efi/libstub/Makefile.zboot
> new file mode 100644
> index 000000000000..38dee29103ae
> --- /dev/null
> +++ b/drivers/firmware/efi/libstub/Makefile.zboot
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# to be include'd by arch/$(ARCH)/boot/Makefile after setting
> +# EFI_ZBOOT_PAYLOAD, EFI_ZBOOT_BFD_TARGET and EFI_ZBOOT_MACH_TYPE
> +
> +comp-type-$(CONFIG_KERNEL_GZIP)		:= gzip
> +comp-type-$(CONFIG_KERNEL_LZ4)		:= lz4
> +comp-type-$(CONFIG_KERNEL_LZMA)		:= lzma
> +comp-type-$(CONFIG_KERNEL_LZO)		:= lzo
> +
> +# in GZIP, the appended le32 carrying the uncompressed size is part of the
> +# format, but in other cases, we just append it at the end for convenience,
> +# causing the original tools to complain when checking image integrity.
> +# So disregard it when calculating the payload size in the zimage header.
> +zimage-method-y				:= $(comp-type-y)_with_size
> +zimage-size-len-y			:= 4
> +
> +zimage-method-$(CONFIG_KERNEL_GZIP)	:= gzip
> +zimage-size-len-$(CONFIG_KERNEL_GZIP)	:= 0
> +
> +quiet_cmd_sbsign = SBSIGN  $@
> +      cmd_sbsign = sbsign --out $@ $< \
> +		   --key $(CONFIG_EFI_ZBOOT_SIGNING_KEY) \
> +		   --cert $(CONFIG_EFI_ZBOOT_SIGNING_CERT) \
> +		   2>/dev/null
> +
> +$(obj)/Image.signed: $(EFI_ZBOOT_PAYLOAD) FORCE
> +	$(call if_changed,sbsign)
> +
> +ZBOOT_PAYLOAD-y				 := $(EFI_ZBOOT_PAYLOAD)
> +ZBOOT_PAYLOAD-$(CONFIG_EFI_ZBOOT_SIGNED) := $(obj)/Image.signed
> +
> +$(obj)/zImage: $(ZBOOT_PAYLOAD-y) FORCE
> +	$(call if_changed,$(zimage-method-y))
> +
> +OBJCOPYFLAGS_zImage.o := -I binary -O $(EFI_ZBOOT_BFD_TARGET) \
> +			 --rename-section .data=.gzdata,load,alloc,readonly,contents
> +$(obj)/zImage.o: $(obj)/zImage FORCE
> +	$(call if_changed,objcopy)
> +
> +AFLAGS_zboot-header.o += -DMACHINE_TYPE=IMAGE_FILE_MACHINE_$(EFI_ZBOOT_MACH_TYPE) \
> +			 -DZIMG_EFI_PATH="\"$(realpath $(obj)/zImage.efi.elf)\"" \
> +			 -DZIMG_SIZE_LEN=$(zimage-size-len-y) \
> +			 -DCOMP_TYPE="\"$(comp-type-y)\""
> +
> +$(obj)/zboot-header.o: $(srctree)/drivers/firmware/efi/libstub/zboot-header.S FORCE
> +	$(call if_changed_rule,as_o_S)
> +
> +ZBOOT_DEPS := $(obj)/zboot-header.o $(objtree)/drivers/firmware/efi/libstub/lib.a
> +
> +LDFLAGS_zImage.efi.elf := -T $(srctree)/drivers/firmware/efi/libstub/zboot.lds
> +$(obj)/zImage.efi.elf: $(obj)/zImage.o $(ZBOOT_DEPS) FORCE
> +	$(call if_changed,ld)
> +
> +ZIMAGE_EFI-y				:= zImage.efi
> +ZIMAGE_EFI-$(CONFIG_EFI_ZBOOT_SIGNED)	:= zImage.efi.unsigned
> +
> +OBJCOPYFLAGS_$(ZIMAGE_EFI-y) := -O binary
> +$(obj)/$(ZIMAGE_EFI-y): $(obj)/zImage.efi.elf FORCE
> +	$(call if_changed,objcopy)
> +
> +targets += zboot-header.o zImage zImage.o zImage.efi.elf zImage.efi
> +
> +ifneq ($(CONFIG_EFI_ZBOOT_SIGNED),)
> +$(obj)/zImage.efi: $(obj)/zImage.efi.unsigned FORCE
> +	$(call if_changed,sbsign)
> +
> +targets += Image.signed zImage.efi.unsigned
> +endif
> diff --git a/drivers/firmware/efi/libstub/zboot-header.S b/drivers/firmware/efi/libstub/zboot-header.S
> new file mode 100644
> index 000000000000..ee6a3cd69773
> --- /dev/null
> +++ b/drivers/firmware/efi/libstub/zboot-header.S
> @@ -0,0 +1,139 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <linux/pe.h>
> +
> +#ifdef CONFIG_64BIT
> +	.set		.Lextra_characteristics, 0x0
> +	.set		.Lpe_opt_magic, PE_OPT_MAGIC_PE32PLUS
> +#else
> +	.set		.Lextra_characteristics, IMAGE_FILE_32BIT_MACHINE
> +	.set		.Lpe_opt_magic, PE_OPT_MAGIC_PE32
> +#endif
> +
> +	.section	".head", "a"
> +	.globl		__efistub_efi_zboot_header
> +__efistub_efi_zboot_header:
> +.Ldoshdr:
> +	.long		MZ_MAGIC
> +	.ascii		"zimg"					// image type
> +	.long		__efistub__gzdata_start - .Ldoshdr	// payload offset
> +	.long		__efistub__gzdata_size - ZIMG_SIZE_LEN	// payload size
> +	.long		0, 0					// reserved
> +	.asciz		COMP_TYPE				// compression type
> +	.org		.Ldoshdr + 0x3c
> +	.long		.Lpehdr - .Ldoshdr			// PE header offset
> +
> +.Lpehdr:
> +	.long		PE_MAGIC
> +	.short		MACHINE_TYPE
> +	.short		.Lsection_count
> +	.long		0
> +	.long		0
> +	.long		0
> +	.short		.Lsection_table - .Loptional_header
> +	.short		IMAGE_FILE_DEBUG_STRIPPED | \
> +			IMAGE_FILE_EXECUTABLE_IMAGE | \
> +			IMAGE_FILE_LINE_NUMS_STRIPPED |\
> +			.Lextra_characteristics
> +
> +.Loptional_header:
> +	.short		.Lpe_opt_magic
> +	.byte		0, 0
> +	.long		_etext - .Lefi_header_end
> +	.long		__data_size
> +	.long		0
> +	.long		__efistub_efi_zboot_entry - .Ldoshdr
> +	.long		.Lefi_header_end - .Ldoshdr
> +
> +#ifdef CONFIG_64BIT
> +	.quad		0
> +#else
> +	.long		_etext - .Ldoshdr, 0x0
> +#endif
> +	.long		4096
> +	.long		512
> +	.short		0, 0
> +	.short		LINUX_EFISTUB_MAJOR_VERSION	// MajorImageVersion
> +	.short		LINUX_EFISTUB_MINOR_VERSION	// MinorImageVersion
> +	.short		0, 0
> +	.long		0
> +	.long		_end - .Ldoshdr
> +
> +	.long		.Lefi_header_end - .Ldoshdr
> +	.long		0
> +	.short		IMAGE_SUBSYSTEM_EFI_APPLICATION
> +	.short		0
> +	.quad		0, 0, 0, 0
> +	.long		0
> +	.long		(.Lsection_table - .) / 8
> +
> +	.quad		0				// ExportTable
> +	.quad		0				// ImportTable
> +	.quad		0				// ResourceTable
> +	.quad		0				// ExceptionTable
> +	.quad		0				// CertificationTable
> +	.quad		0				// BaseRelocationTable
> +#ifdef CONFIG_DEBUG_EFI
> +	.long		.Lefi_debug_table - .Ldoshdr	// DebugTable
> +	.long		.Lefi_debug_table_size
> +#endif
> +
> +.Lsection_table:
> +	.ascii		".text\0\0\0"
> +	.long		_etext - .Lefi_header_end
> +	.long		.Lefi_header_end - .Ldoshdr
> +	.long		_etext - .Lefi_header_end
> +	.long		.Lefi_header_end - .Ldoshdr
> +
> +	.long		0, 0
> +	.short		0, 0
> +	.long		IMAGE_SCN_CNT_CODE | \
> +			IMAGE_SCN_MEM_READ | \
> +			IMAGE_SCN_MEM_EXECUTE
> +
> +	.ascii		".data\0\0\0"
> +	.long		__data_size
> +	.long		_etext - .Ldoshdr
> +	.long		__data_rawsize
> +	.long		_etext - .Ldoshdr
> +
> +	.long		0, 0
> +	.short		0, 0
> +	.long		IMAGE_SCN_CNT_INITIALIZED_DATA | \
> +			IMAGE_SCN_MEM_READ | \
> +			IMAGE_SCN_MEM_WRITE
> +
> +	.set		.Lsection_count, (. - .Lsection_table) / 40
> +
> +#ifdef CONFIG_DEBUG_EFI
> +	.section	".rodata", "a"
> +	.align		2
> +.Lefi_debug_table:
> +	// EFI_IMAGE_DEBUG_DIRECTORY_ENTRY
> +	.long		0				// Characteristics
> +	.long		0				// TimeDateStamp
> +	.short		0				// MajorVersion
> +	.short		0				// MinorVersion
> +	.long		IMAGE_DEBUG_TYPE_CODEVIEW	// Type
> +	.long		.Lefi_debug_entry_size		// SizeOfData
> +	.long		0				// RVA
> +	.long		.Lefi_debug_entry - .Ldoshdr	// FileOffset
> +
> +	.set		.Lefi_debug_table_size, . - .Lefi_debug_table
> +	.previous
> +
> +.Lefi_debug_entry:
> +	// EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY
> +	.ascii		"NB10"				// Signature
> +	.long		0				// Unknown
> +	.long		0				// Unknown2
> +	.long		0				// Unknown3
> +
> +	.asciz		ZIMG_EFI_PATH
> +
> +	.set		.Lefi_debug_entry_size, . - .Lefi_debug_entry
> +#endif
> +
> +	.p2align	12
> +.Lefi_header_end:
> +
> diff --git a/drivers/firmware/efi/libstub/zboot.c b/drivers/firmware/efi/libstub/zboot.c
> new file mode 100644
> index 000000000000..9cf968e90775
> --- /dev/null
> +++ b/drivers/firmware/efi/libstub/zboot.c
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/efi.h>
> +#include <linux/pe.h>
> +#include <asm/efi.h>
> +
> +#include "efistub.h"
> +
> +static unsigned char zboot_heap[SZ_64K] __aligned(64);
> +static unsigned long free_mem_ptr, free_mem_end_ptr;
> +
> +#define STATIC static
> +#if defined(CONFIG_KERNEL_GZIP)
> +#include "../../../../lib/decompress_inflate.c"
> +#elif defined(CONFIG_KERNEL_LZ4)
> +#include "../../../../lib/decompress_unlz4.c"
> +#elif defined(CONFIG_KERNEL_LZMA)
> +#include "../../../../lib/decompress_unlzma.c"
> +#elif defined(CONFIG_KERNEL_LZO)
> +#include "../../../../lib/decompress_unlzo.c"
> +#endif
> +
> +extern char _gzdata_start[], _gzdata_end[];
> +extern u32 uncompressed_size __aligned(1);
> +
> +static void log(efi_char16_t str[])
> +{
> +	efi_call_proto(efi_table_attr(efi_system_table, con_out),
> +		       output_string, L"EFI decompressor: ");
> +	efi_call_proto(efi_table_attr(efi_system_table, con_out),
> +		       output_string, str);
> +}
> +
> +static void error(char *x)
> +{
> +	log(L"error() called from decompressor library\n");
> +}
> +
> +efi_status_t __efiapi efi_zboot_entry(efi_handle_t handle,
> +				      efi_system_table_t *systab)
> +{
> +	static efi_guid_t loaded_image = LOADED_IMAGE_PROTOCOL_GUID;
> +	efi_loaded_image_t *parent, *child;
> +	unsigned long image_buffer;
> +	efi_handle_t child_handle;
> +	efi_status_t status;
> +	int ret;
> +
> +	WRITE_ONCE(efi_system_table, systab);
> +
> +	free_mem_ptr = (unsigned long)&zboot_heap;
> +	free_mem_end_ptr = free_mem_ptr + sizeof(zboot_heap);
> +
> +	status = efi_bs_call(handle_protocol, handle, &loaded_image,
> +			     (void **)&parent);
> +	if (status != EFI_SUCCESS) {
> +		log(L"Failed to locate parent's loaded image protocol\n");
> +		return status;
> +	}
> +
> +	status = efi_allocate_pages(uncompressed_size, &image_buffer, ULONG_MAX);
> +	if (status != EFI_SUCCESS) {
> +		log(L"Failed to allocate memory\n");
> +		return status;
> +	}
> +
> +	ret = __decompress(_gzdata_start, _gzdata_end - _gzdata_start, NULL,
> +			   NULL, (unsigned char *)image_buffer, 0, NULL,
> +			   error);
> +	if (ret	< 0) {
> +		log(L"Decompression failed\n");
> +		return EFI_LOAD_ERROR;
> +	}
> +
> +	status = efi_bs_call(load_image, false, handle, NULL,

I would prefer to pass the device path of the compressed image instead 
of NULL. This way information is not lost.

> +			     (void *)image_buffer, uncompressed_size,
> +			     &child_handle);
> +	if (status != EFI_SUCCESS) {
> +		log(L"Failed to load image\n");
> +		return status;
> +	}
> +
> +	status = efi_bs_call(handle_protocol, child_handle, &loaded_image,
> +			     (void **)&child);
> +	if (status != EFI_SUCCESS) {
> +		log(L"Failed to locate child's loaded image protocol\n");
> +		return status;
> +	}
> +
> +	// Copy the kernel command line
> +	child->load_options = parent->load_options;
> +	child->load_options_size = parent->load_options_size;
> +
> +	status = efi_bs_call(start_image, child_handle, NULL, NULL);
> +	if (status != EFI_SUCCESS) {
> +		log(L"StartImage() returned with error\n");

Please, pass pointers for ExitDataSize and ExitData. If ExitDataSize != 
0 a string is provided in ExitData. Return that data to the caller of 
the compressed image. You may additionally print the string here.

The caller then will then take care of freeing ExitData (via FreePool()) 
and optionally log the information.

Best regards

Heinrich

> +		return status;
> +	}
> +
> +	return EFI_SUCCESS;
> +}
> diff --git a/drivers/firmware/efi/libstub/zboot.lds b/drivers/firmware/efi/libstub/zboot.lds
> new file mode 100644
> index 000000000000..d6ba89a0c294
> --- /dev/null
> +++ b/drivers/firmware/efi/libstub/zboot.lds
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +ENTRY(__efistub_efi_zboot_header);
> +
> +SECTIONS
> +{
> +	.text : ALIGN(4096) {
> +		*(.head)
> +		*(.text* .init.text*)
> +	}
> +
> +	.rodata : ALIGN(8) {
> +		__efistub__gzdata_start = .;
> +		*(.gzdata)
> +		__efistub__gzdata_end = .;
> +		*(.rodata* .init.rodata* .srodata*)
> +		_etext = ALIGN(4096);
> +		. = _etext;
> +	}
> +
> +	.data : ALIGN(4096) {
> +		*(.data* .init.data*)
> +		_edata = ALIGN(512);
> +		. = _edata;
> +	}
> +
> +	.bss : {
> +		*(.bss* .init.bss*)
> +		_end = ALIGN(512);
> +		. = _end;
> +	}
> +}
> +
> +PROVIDE(__efistub__gzdata_size = ABSOLUTE(. - __efistub__gzdata_start));
> +
> +PROVIDE(__efistub_uncompressed_size = __efistub__gzdata_end - 4);
> +
> +PROVIDE(__data_rawsize = ABSOLUTE(_edata - _etext));
> +PROVIDE(__data_size = ABSOLUTE(_end - _etext));

Re: [PATCH v3 4/6] efi/libstub: implement generic EFI zboot

[Ard Biesheuvel]

On Thu, 18 Aug 2022 at 18:42, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> On 8/17/22 13:03, Ard Biesheuvel wrote:
> > Implement a minimal EFI app that decompresses the real kernel image and
> > launches it using the firmware's LoadImage and StartImage boot services.
> > This removes the need for any arch-specific hacks.
> >
> > Note that on systems that have UEFI secure boot policies enabled,
> > LoadImage/StartImage require images to be signed, or their hashes known
> > a priori, in order to be permitted to boot.
> >
> > There are various possible strategies to work around this requirement,
> > but they all rely either on overriding internal PI/DXE protocols (which
> > are not part of the EFI spec) or omitting the firmware provided
> > LoadImage() and StartImage() boot services, which is also undesirable,
> > given that they encapsulate platform specific policies related to secure
> > boot and measured boot, but also related to memory permissions (whether
> > or not and which types of heap allocations have both write and execute
> > permissions.)
> >
> > The only generic and truly portable way around this is to simply sign
> > both the inner and the outer image with the same key/cert pair, so this
> > is what is implemented here.
> >
> > BZIP2 has been omitted from the set of supported compression algorithms,
> > given that its performance is mediocre both in speed and size, and it
> > uses a disproportionate amount of memory. For optimal compression, use
> > LZMA. For the fastest boot speed, use LZO.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >   drivers/firmware/efi/Kconfig                |  31 ++++-
> >   drivers/firmware/efi/libstub/Makefile       |   8 +-
> >   drivers/firmware/efi/libstub/Makefile.zboot |  69 ++++++++++
> >   drivers/firmware/efi/libstub/zboot-header.S | 139 ++++++++++++++++++++
> >   drivers/firmware/efi/libstub/zboot.c        | 101 ++++++++++++++
> >   drivers/firmware/efi/libstub/zboot.lds      |  39 ++++++
> >   6 files changed, 382 insertions(+), 5 deletions(-)
> >
...
> > diff --git a/drivers/firmware/efi/libstub/zboot.c b/drivers/firmware/efi/libstub/zboot.c
> > new file mode 100644
> > index 000000000000..9cf968e90775
> > --- /dev/null
> > +++ b/drivers/firmware/efi/libstub/zboot.c
...
> > +efi_status_t __efiapi efi_zboot_entry(efi_handle_t handle,
> > +                                   efi_system_table_t *systab)
> > +{
> > +     static efi_guid_t loaded_image = LOADED_IMAGE_PROTOCOL_GUID;
> > +     efi_loaded_image_t *parent, *child;
> > +     unsigned long image_buffer;
> > +     efi_handle_t child_handle;
> > +     efi_status_t status;
> > +     int ret;
> > +
> > +     WRITE_ONCE(efi_system_table, systab);
> > +
> > +     free_mem_ptr = (unsigned long)&zboot_heap;
> > +     free_mem_end_ptr = free_mem_ptr + sizeof(zboot_heap);
> > +
> > +     status = efi_bs_call(handle_protocol, handle, &loaded_image,
> > +                          (void **)&parent);
> > +     if (status != EFI_SUCCESS) {
> > +             log(L"Failed to locate parent's loaded image protocol\n");
> > +             return status;
> > +     }
> > +
> > +     status = efi_allocate_pages(uncompressed_size, &image_buffer, ULONG_MAX);
> > +     if (status != EFI_SUCCESS) {
> > +             log(L"Failed to allocate memory\n");
> > +             return status;
> > +     }
> > +
> > +     ret = __decompress(_gzdata_start, _gzdata_end - _gzdata_start, NULL,
> > +                        NULL, (unsigned char *)image_buffer, 0, NULL,
> > +                        error);
> > +     if (ret < 0) {
> > +             log(L"Decompression failed\n");
> > +             return EFI_LOAD_ERROR;
> > +     }
> > +
> > +     status = efi_bs_call(load_image, false, handle, NULL,
>
> I would prefer to pass the device path of the compressed image instead
> of NULL. This way information is not lost.
>

That way, we will have two loaded images with different handles
claiming to be loaded from the same device path - I don't think that
is appropriate tbh.

What we could do is define a vendor GUID for the decompressed kernel,
and create a device path for it. That way, you can grab the
loaded_image of the parent to obtain this information.

What did you have in mind as a use case?

> > +                          (void *)image_buffer, uncompressed_size,
> > +                          &child_handle);
> > +     if (status != EFI_SUCCESS) {
> > +             log(L"Failed to load image\n");
> > +             return status;
> > +     }
> > +
> > +     status = efi_bs_call(handle_protocol, child_handle, &loaded_image,
> > +                          (void **)&child);
> > +     if (status != EFI_SUCCESS) {
> > +             log(L"Failed to locate child's loaded image protocol\n");
> > +             return status;
> > +     }
> > +
> > +     // Copy the kernel command line
> > +     child->load_options = parent->load_options;
> > +     child->load_options_size = parent->load_options_size;
> > +
> > +     status = efi_bs_call(start_image, child_handle, NULL, NULL);
> > +     if (status != EFI_SUCCESS) {
> > +             log(L"StartImage() returned with error\n");
>
> Please, pass pointers for ExitDataSize and ExitData. If ExitDataSize !=
> 0 a string is provided in ExitData. Return that data to the caller of
> the compressed image. You may additionally print the string here.
>
> The caller then will then take care of freeing ExitData (via FreePool())
> and optionally log the information.
>

Good idea, I will add that.

Thanks,
Ard.

Re: [PATCH v3 4/6] efi/libstub: implement generic EFI zboot

[Heinrich Schuchardt]

On 8/18/22 19:10, Ard Biesheuvel wrote:
> On Thu, 18 Aug 2022 at 18:42, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> On 8/17/22 13:03, Ard Biesheuvel wrote:
>>> Implement a minimal EFI app that decompresses the real kernel image and
>>> launches it using the firmware's LoadImage and StartImage boot services.
>>> This removes the need for any arch-specific hacks.
>>>
>>> Note that on systems that have UEFI secure boot policies enabled,
>>> LoadImage/StartImage require images to be signed, or their hashes known
>>> a priori, in order to be permitted to boot.
>>>
>>> There are various possible strategies to work around this requirement,
>>> but they all rely either on overriding internal PI/DXE protocols (which
>>> are not part of the EFI spec) or omitting the firmware provided
>>> LoadImage() and StartImage() boot services, which is also undesirable,
>>> given that they encapsulate platform specific policies related to secure
>>> boot and measured boot, but also related to memory permissions (whether
>>> or not and which types of heap allocations have both write and execute
>>> permissions.)
>>>
>>> The only generic and truly portable way around this is to simply sign
>>> both the inner and the outer image with the same key/cert pair, so this
>>> is what is implemented here.
>>>
>>> BZIP2 has been omitted from the set of supported compression algorithms,
>>> given that its performance is mediocre both in speed and size, and it
>>> uses a disproportionate amount of memory. For optimal compression, use
>>> LZMA. For the fastest boot speed, use LZO.
>>>
>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>> ---
>>>    drivers/firmware/efi/Kconfig                |  31 ++++-
>>>    drivers/firmware/efi/libstub/Makefile       |   8 +-
>>>    drivers/firmware/efi/libstub/Makefile.zboot |  69 ++++++++++
>>>    drivers/firmware/efi/libstub/zboot-header.S | 139 ++++++++++++++++++++
>>>    drivers/firmware/efi/libstub/zboot.c        | 101 ++++++++++++++
>>>    drivers/firmware/efi/libstub/zboot.lds      |  39 ++++++
>>>    6 files changed, 382 insertions(+), 5 deletions(-)
>>>
> ...
>>> diff --git a/drivers/firmware/efi/libstub/zboot.c b/drivers/firmware/efi/libstub/zboot.c
>>> new file mode 100644
>>> index 000000000000..9cf968e90775
>>> --- /dev/null
>>> +++ b/drivers/firmware/efi/libstub/zboot.c
> ...
>>> +efi_status_t __efiapi efi_zboot_entry(efi_handle_t handle,
>>> +                                   efi_system_table_t *systab)
>>> +{
>>> +     static efi_guid_t loaded_image = LOADED_IMAGE_PROTOCOL_GUID;
>>> +     efi_loaded_image_t *parent, *child;
>>> +     unsigned long image_buffer;
>>> +     efi_handle_t child_handle;
>>> +     efi_status_t status;
>>> +     int ret;
>>> +
>>> +     WRITE_ONCE(efi_system_table, systab);
>>> +
>>> +     free_mem_ptr = (unsigned long)&zboot_heap;
>>> +     free_mem_end_ptr = free_mem_ptr + sizeof(zboot_heap);
>>> +
>>> +     status = efi_bs_call(handle_protocol, handle, &loaded_image,
>>> +                          (void **)&parent);
>>> +     if (status != EFI_SUCCESS) {
>>> +             log(L"Failed to locate parent's loaded image protocol\n");
>>> +             return status;
>>> +     }
>>> +
>>> +     status = efi_allocate_pages(uncompressed_size, &image_buffer, ULONG_MAX);
>>> +     if (status != EFI_SUCCESS) {
>>> +             log(L"Failed to allocate memory\n");
>>> +             return status;
>>> +     }
>>> +
>>> +     ret = __decompress(_gzdata_start, _gzdata_end - _gzdata_start, NULL,
>>> +                        NULL, (unsigned char *)image_buffer, 0, NULL,
>>> +                        error);
>>> +     if (ret < 0) {
>>> +             log(L"Decompression failed\n");
>>> +             return EFI_LOAD_ERROR;
>>> +     }
>>> +
>>> +     status = efi_bs_call(load_image, false, handle, NULL,
>>
>> I would prefer to pass the device path of the compressed image instead
>> of NULL. This way information is not lost.
>>
> 
> That way, we will have two loaded images with different handles
> claiming to be loaded from the same device path - I don't think that
> is appropriate tbh.

They both are the product of the same file on disk.

> 
> What we could do is define a vendor GUID for the decompressed kernel,
> and create a device path for it. That way, you can grab the
> loaded_image of the parent to obtain this information.
> 
> What did you have in mind as a use case?

The device-path could be used in the kernel log.

It can be used to find the device or folder with initrd where we use 
initrd= on the command line.

You could use the device path to access the original file, e.g. to read 
additional information.

For all use cases you would want to have the original device path.

Best regards

Heinrich

> 
>>> +                          (void *)image_buffer, uncompressed_size,
>>> +                          &child_handle);
>>> +     if (status != EFI_SUCCESS) {
>>> +             log(L"Failed to load image\n");
>>> +             return status;
>>> +     }
>>> +
>>> +     status = efi_bs_call(handle_protocol, child_handle, &loaded_image,
>>> +                          (void **)&child);
>>> +     if (status != EFI_SUCCESS) {
>>> +             log(L"Failed to locate child's loaded image protocol\n");
>>> +             return status;
>>> +     }
>>> +
>>> +     // Copy the kernel command line
>>> +     child->load_options = parent->load_options;
>>> +     child->load_options_size = parent->load_options_size;
>>> +
>>> +     status = efi_bs_call(start_image, child_handle, NULL, NULL);
>>> +     if (status != EFI_SUCCESS) {
>>> +             log(L"StartImage() returned with error\n");
>>
>> Please, pass pointers for ExitDataSize and ExitData. If ExitDataSize !=
>> 0 a string is provided in ExitData. Return that data to the caller of
>> the compressed image. You may additionally print the string here.
>>
>> The caller then will then take care of freeing ExitData (via FreePool())
>> and optionally log the information.
>>
> 
> Good idea, I will add that.
> 
> Thanks,
> Ard.

Re: [PATCH v3 4/6] efi/libstub: implement generic EFI zboot

[Ard Biesheuvel]

On Fri, 19 Aug 2022 at 07:29, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
>
>
> On 8/18/22 19:10, Ard Biesheuvel wrote:
> > On Thu, 18 Aug 2022 at 18:42, Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> >>
> >> On 8/17/22 13:03, Ard Biesheuvel wrote:
> >>> Implement a minimal EFI app that decompresses the real kernel image and
> >>> launches it using the firmware's LoadImage and StartImage boot services.
> >>> This removes the need for any arch-specific hacks.
> >>>
> >>> Note that on systems that have UEFI secure boot policies enabled,
> >>> LoadImage/StartImage require images to be signed, or their hashes known
> >>> a priori, in order to be permitted to boot.
> >>>
> >>> There are various possible strategies to work around this requirement,
> >>> but they all rely either on overriding internal PI/DXE protocols (which
> >>> are not part of the EFI spec) or omitting the firmware provided
> >>> LoadImage() and StartImage() boot services, which is also undesirable,
> >>> given that they encapsulate platform specific policies related to secure
> >>> boot and measured boot, but also related to memory permissions (whether
> >>> or not and which types of heap allocations have both write and execute
> >>> permissions.)
> >>>
> >>> The only generic and truly portable way around this is to simply sign
> >>> both the inner and the outer image with the same key/cert pair, so this
> >>> is what is implemented here.
> >>>
> >>> BZIP2 has been omitted from the set of supported compression algorithms,
> >>> given that its performance is mediocre both in speed and size, and it
> >>> uses a disproportionate amount of memory. For optimal compression, use
> >>> LZMA. For the fastest boot speed, use LZO.
> >>>
> >>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >>> ---
> >>>    drivers/firmware/efi/Kconfig                |  31 ++++-
> >>>    drivers/firmware/efi/libstub/Makefile       |   8 +-
> >>>    drivers/firmware/efi/libstub/Makefile.zboot |  69 ++++++++++
> >>>    drivers/firmware/efi/libstub/zboot-header.S | 139 ++++++++++++++++++++
> >>>    drivers/firmware/efi/libstub/zboot.c        | 101 ++++++++++++++
> >>>    drivers/firmware/efi/libstub/zboot.lds      |  39 ++++++
> >>>    6 files changed, 382 insertions(+), 5 deletions(-)
> >>>
> > ...
> >>> diff --git a/drivers/firmware/efi/libstub/zboot.c b/drivers/firmware/efi/libstub/zboot.c
> >>> new file mode 100644
> >>> index 000000000000..9cf968e90775
> >>> --- /dev/null
> >>> +++ b/drivers/firmware/efi/libstub/zboot.c
> > ...
> >>> +efi_status_t __efiapi efi_zboot_entry(efi_handle_t handle,
> >>> +                                   efi_system_table_t *systab)
> >>> +{
> >>> +     static efi_guid_t loaded_image = LOADED_IMAGE_PROTOCOL_GUID;
> >>> +     efi_loaded_image_t *parent, *child;
> >>> +     unsigned long image_buffer;
> >>> +     efi_handle_t child_handle;
> >>> +     efi_status_t status;
> >>> +     int ret;
> >>> +
> >>> +     WRITE_ONCE(efi_system_table, systab);
> >>> +
> >>> +     free_mem_ptr = (unsigned long)&zboot_heap;
> >>> +     free_mem_end_ptr = free_mem_ptr + sizeof(zboot_heap);
> >>> +
> >>> +     status = efi_bs_call(handle_protocol, handle, &loaded_image,
> >>> +                          (void **)&parent);
> >>> +     if (status != EFI_SUCCESS) {
> >>> +             log(L"Failed to locate parent's loaded image protocol\n");
> >>> +             return status;
> >>> +     }
> >>> +
> >>> +     status = efi_allocate_pages(uncompressed_size, &image_buffer, ULONG_MAX);
> >>> +     if (status != EFI_SUCCESS) {
> >>> +             log(L"Failed to allocate memory\n");
> >>> +             return status;
> >>> +     }
> >>> +
> >>> +     ret = __decompress(_gzdata_start, _gzdata_end - _gzdata_start, NULL,
> >>> +                        NULL, (unsigned char *)image_buffer, 0, NULL,
> >>> +                        error);
> >>> +     if (ret < 0) {
> >>> +             log(L"Decompression failed\n");
> >>> +             return EFI_LOAD_ERROR;
> >>> +     }
> >>> +
> >>> +     status = efi_bs_call(load_image, false, handle, NULL,
> >>
> >> I would prefer to pass the device path of the compressed image instead
> >> of NULL. This way information is not lost.
> >>
> >
> > That way, we will have two loaded images with different handles
> > claiming to be loaded from the same device path - I don't think that
> > is appropriate tbh.
>
> They both are the product of the same file on disk.
>

But they are not the same. When re-loading the device path (as you
suggest below) you will get a completely different file, and the only
way to get at the payload is to execute it.

So using the same device path is out of the question imo.

> >
> > What we could do is define a vendor GUID for the decompressed kernel,
> > and create a device path for it. That way, you can grab the
> > loaded_image of the parent to obtain this information.
> >
> > What did you have in mind as a use case?
>
> The device-path could be used in the kernel log.
>
> It can be used to find the device or folder with initrd where we use
> initrd= on the command line.
>
> You could use the device path to access the original file, e.g. to read
> additional information.
>
> For all use cases you would want to have the original device path.
>

What we could do is:

- define a device path in the decompressor, e.g.,

<original device path>/Offset(<start>, <end>)/VendorMedia(xxx-xxx-xxx,
<compression type>)

where start, end and compression type describe the compressed payload
inside the decompressor executable. (The compression type could be
omitted, or could be a separate node.)

- install the LoadFile2 protocol and the device path protocol onto a
handle, and move the decompression logic into the LoadFile2
implementation

- drop the SourceBuffer and SourceSize arguments to LoadImage(), and
pass the device path instead, so that LoadFile2 will be invoked by
LoadImage directly to perform the decompression.

That way, we retain the information about the outer file, and each
piece is described in detail in device path notation. As a bonus, we
could easily expose the compressed part separately, if there is a need
for that.

This doesn't cover the initrd= issue you raised, but that is something
we could address later in the stub if we wanted to (but I don't think
initrd= is something we should care too much about)

Re: [PATCH v3 4/6] efi/libstub: implement generic EFI zboot

[Heinrich Schuchardt]

On 8/19/22 08:52, Ard Biesheuvel wrote:
> On Fri, 19 Aug 2022 at 07:29, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>>
>>
>> On 8/18/22 19:10, Ard Biesheuvel wrote:
>>> On Thu, 18 Aug 2022 at 18:42, Heinrich Schuchardt
>>> <heinrich.schuchardt@canonical.com> wrote:
>>>>
>>>> On 8/17/22 13:03, Ard Biesheuvel wrote:
>>>>> Implement a minimal EFI app that decompresses the real kernel image and
>>>>> launches it using the firmware's LoadImage and StartImage boot services.
>>>>> This removes the need for any arch-specific hacks.
>>>>>
>>>>> Note that on systems that have UEFI secure boot policies enabled,
>>>>> LoadImage/StartImage require images to be signed, or their hashes known
>>>>> a priori, in order to be permitted to boot.
>>>>>
>>>>> There are various possible strategies to work around this requirement,
>>>>> but they all rely either on overriding internal PI/DXE protocols (which
>>>>> are not part of the EFI spec) or omitting the firmware provided
>>>>> LoadImage() and StartImage() boot services, which is also undesirable,
>>>>> given that they encapsulate platform specific policies related to secure
>>>>> boot and measured boot, but also related to memory permissions (whether
>>>>> or not and which types of heap allocations have both write and execute
>>>>> permissions.)
>>>>>
>>>>> The only generic and truly portable way around this is to simply sign
>>>>> both the inner and the outer image with the same key/cert pair, so this
>>>>> is what is implemented here.
>>>>>
>>>>> BZIP2 has been omitted from the set of supported compression algorithms,
>>>>> given that its performance is mediocre both in speed and size, and it
>>>>> uses a disproportionate amount of memory. For optimal compression, use
>>>>> LZMA. For the fastest boot speed, use LZO.
>>>>>
>>>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>>>> ---
>>>>>     drivers/firmware/efi/Kconfig                |  31 ++++-
>>>>>     drivers/firmware/efi/libstub/Makefile       |   8 +-
>>>>>     drivers/firmware/efi/libstub/Makefile.zboot |  69 ++++++++++
>>>>>     drivers/firmware/efi/libstub/zboot-header.S | 139 ++++++++++++++++++++
>>>>>     drivers/firmware/efi/libstub/zboot.c        | 101 ++++++++++++++
>>>>>     drivers/firmware/efi/libstub/zboot.lds      |  39 ++++++
>>>>>     6 files changed, 382 insertions(+), 5 deletions(-)
>>>>>
>>> ...
>>>>> diff --git a/drivers/firmware/efi/libstub/zboot.c b/drivers/firmware/efi/libstub/zboot.c
>>>>> new file mode 100644
>>>>> index 000000000000..9cf968e90775
>>>>> --- /dev/null
>>>>> +++ b/drivers/firmware/efi/libstub/zboot.c
>>> ...
>>>>> +efi_status_t __efiapi efi_zboot_entry(efi_handle_t handle,
>>>>> +                                   efi_system_table_t *systab)
>>>>> +{
>>>>> +     static efi_guid_t loaded_image = LOADED_IMAGE_PROTOCOL_GUID;
>>>>> +     efi_loaded_image_t *parent, *child;
>>>>> +     unsigned long image_buffer;
>>>>> +     efi_handle_t child_handle;
>>>>> +     efi_status_t status;
>>>>> +     int ret;
>>>>> +
>>>>> +     WRITE_ONCE(efi_system_table, systab);
>>>>> +
>>>>> +     free_mem_ptr = (unsigned long)&zboot_heap;
>>>>> +     free_mem_end_ptr = free_mem_ptr + sizeof(zboot_heap);
>>>>> +
>>>>> +     status = efi_bs_call(handle_protocol, handle, &loaded_image,
>>>>> +                          (void **)&parent);
>>>>> +     if (status != EFI_SUCCESS) {
>>>>> +             log(L"Failed to locate parent's loaded image protocol\n");
>>>>> +             return status;
>>>>> +     }
>>>>> +
>>>>> +     status = efi_allocate_pages(uncompressed_size, &image_buffer, ULONG_MAX);
>>>>> +     if (status != EFI_SUCCESS) {
>>>>> +             log(L"Failed to allocate memory\n");
>>>>> +             return status;
>>>>> +     }
>>>>> +
>>>>> +     ret = __decompress(_gzdata_start, _gzdata_end - _gzdata_start, NULL,
>>>>> +                        NULL, (unsigned char *)image_buffer, 0, NULL,
>>>>> +                        error);
>>>>> +     if (ret < 0) {
>>>>> +             log(L"Decompression failed\n");
>>>>> +             return EFI_LOAD_ERROR;
>>>>> +     }
>>>>> +
>>>>> +     status = efi_bs_call(load_image, false, handle, NULL,
>>>>
>>>> I would prefer to pass the device path of the compressed image instead
>>>> of NULL. This way information is not lost.
>>>>
>>>
>>> That way, we will have two loaded images with different handles
>>> claiming to be loaded from the same device path - I don't think that
>>> is appropriate tbh.
>>
>> They both are the product of the same file on disk.
>>
> 
> But they are not the same. When re-loading the device path (as you
> suggest below) you will get a completely different file, and the only
> way to get at the payload is to execute it.
> 
> So using the same device path is out of the question imo.

How about appending a VenHW() node with a decompressor specific GUID at 
the end of the DP?

I think that is the most UEFIish way to express that the handle is 
derived from the compressed file.

You could even put additional information into the VenHW() node like the 
compression type or the compressed size.

Best regards

Heinrich

> 
>>>
>>> What we could do is define a vendor GUID for the decompressed kernel,
>>> and create a device path for it. That way, you can grab the
>>> loaded_image of the parent to obtain this information.
>>>
>>> What did you have in mind as a use case?
>>
>> The device-path could be used in the kernel log.
>>
>> It can be used to find the device or folder with initrd where we use
>> initrd= on the command line.
>>
>> You could use the device path to access the original file, e.g. to read
>> additional information.
>>
>> For all use cases you would want to have the original device path.
>>
> 
> What we could do is:
> 
> - define a device path in the decompressor, e.g.,
> 
> <original device path>/Offset(<start>, <end>)/VendorMedia(xxx-xxx-xxx,
> <compression type>)
> 
> where start, end and compression type describe the compressed payload
> inside the decompressor executable. (The compression type could be
> omitted, or could be a separate node.)
> 
> - install the LoadFile2 protocol and the device path protocol onto a
> handle, and move the decompression logic into the LoadFile2
> implementation
> 
> - drop the SourceBuffer and SourceSize arguments to LoadImage(), and
> pass the device path instead, so that LoadFile2 will be invoked by
> LoadImage directly to perform the decompression.
> 
> That way, we retain the information about the outer file, and each
> piece is described in detail in device path notation. As a bonus, we
> could easily expose the compressed part separately, if there is a need
> for that.
> 
> This doesn't cover the initrd= issue you raised, but that is something
> we could address later in the stub if we wanted to (but I don't think
> initrd= is something we should care too much about)

Re: [PATCH v3 4/6] efi/libstub: implement generic EFI zboot

[Ard Biesheuvel]

On Fri, 19 Aug 2022 at 09:01, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> On 8/19/22 08:52, Ard Biesheuvel wrote:
> > On Fri, 19 Aug 2022 at 07:29, Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> >>
> >>
> >>
> >> On 8/18/22 19:10, Ard Biesheuvel wrote:
> >>> On Thu, 18 Aug 2022 at 18:42, Heinrich Schuchardt
> >>> <heinrich.schuchardt@canonical.com> wrote:
> >>>>
> >>>> On 8/17/22 13:03, Ard Biesheuvel wrote:
> >>>>> Implement a minimal EFI app that decompresses the real kernel image and
> >>>>> launches it using the firmware's LoadImage and StartImage boot services.
> >>>>> This removes the need for any arch-specific hacks.
> >>>>>
> >>>>> Note that on systems that have UEFI secure boot policies enabled,
> >>>>> LoadImage/StartImage require images to be signed, or their hashes known
> >>>>> a priori, in order to be permitted to boot.
> >>>>>
> >>>>> There are various possible strategies to work around this requirement,
> >>>>> but they all rely either on overriding internal PI/DXE protocols (which
> >>>>> are not part of the EFI spec) or omitting the firmware provided
> >>>>> LoadImage() and StartImage() boot services, which is also undesirable,
> >>>>> given that they encapsulate platform specific policies related to secure
> >>>>> boot and measured boot, but also related to memory permissions (whether
> >>>>> or not and which types of heap allocations have both write and execute
> >>>>> permissions.)
> >>>>>
> >>>>> The only generic and truly portable way around this is to simply sign
> >>>>> both the inner and the outer image with the same key/cert pair, so this
> >>>>> is what is implemented here.
> >>>>>
> >>>>> BZIP2 has been omitted from the set of supported compression algorithms,
> >>>>> given that its performance is mediocre both in speed and size, and it
> >>>>> uses a disproportionate amount of memory. For optimal compression, use
> >>>>> LZMA. For the fastest boot speed, use LZO.
> >>>>>
> >>>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >>>>> ---
> >>>>>     drivers/firmware/efi/Kconfig                |  31 ++++-
> >>>>>     drivers/firmware/efi/libstub/Makefile       |   8 +-
> >>>>>     drivers/firmware/efi/libstub/Makefile.zboot |  69 ++++++++++
> >>>>>     drivers/firmware/efi/libstub/zboot-header.S | 139 ++++++++++++++++++++
> >>>>>     drivers/firmware/efi/libstub/zboot.c        | 101 ++++++++++++++
> >>>>>     drivers/firmware/efi/libstub/zboot.lds      |  39 ++++++
> >>>>>     6 files changed, 382 insertions(+), 5 deletions(-)
> >>>>>
> >>> ...
> >>>>> diff --git a/drivers/firmware/efi/libstub/zboot.c b/drivers/firmware/efi/libstub/zboot.c
> >>>>> new file mode 100644
> >>>>> index 000000000000..9cf968e90775
> >>>>> --- /dev/null
> >>>>> +++ b/drivers/firmware/efi/libstub/zboot.c
> >>> ...
> >>>>> +efi_status_t __efiapi efi_zboot_entry(efi_handle_t handle,
> >>>>> +                                   efi_system_table_t *systab)
> >>>>> +{
> >>>>> +     static efi_guid_t loaded_image = LOADED_IMAGE_PROTOCOL_GUID;
> >>>>> +     efi_loaded_image_t *parent, *child;
> >>>>> +     unsigned long image_buffer;
> >>>>> +     efi_handle_t child_handle;
> >>>>> +     efi_status_t status;
> >>>>> +     int ret;
> >>>>> +
> >>>>> +     WRITE_ONCE(efi_system_table, systab);
> >>>>> +
> >>>>> +     free_mem_ptr = (unsigned long)&zboot_heap;
> >>>>> +     free_mem_end_ptr = free_mem_ptr + sizeof(zboot_heap);
> >>>>> +
> >>>>> +     status = efi_bs_call(handle_protocol, handle, &loaded_image,
> >>>>> +                          (void **)&parent);
> >>>>> +     if (status != EFI_SUCCESS) {
> >>>>> +             log(L"Failed to locate parent's loaded image protocol\n");
> >>>>> +             return status;
> >>>>> +     }
> >>>>> +
> >>>>> +     status = efi_allocate_pages(uncompressed_size, &image_buffer, ULONG_MAX);
> >>>>> +     if (status != EFI_SUCCESS) {
> >>>>> +             log(L"Failed to allocate memory\n");
> >>>>> +             return status;
> >>>>> +     }
> >>>>> +
> >>>>> +     ret = __decompress(_gzdata_start, _gzdata_end - _gzdata_start, NULL,
> >>>>> +                        NULL, (unsigned char *)image_buffer, 0, NULL,
> >>>>> +                        error);
> >>>>> +     if (ret < 0) {
> >>>>> +             log(L"Decompression failed\n");
> >>>>> +             return EFI_LOAD_ERROR;
> >>>>> +     }
> >>>>> +
> >>>>> +     status = efi_bs_call(load_image, false, handle, NULL,
> >>>>
> >>>> I would prefer to pass the device path of the compressed image instead
> >>>> of NULL. This way information is not lost.
> >>>>
> >>>
> >>> That way, we will have two loaded images with different handles
> >>> claiming to be loaded from the same device path - I don't think that
> >>> is appropriate tbh.
> >>
> >> They both are the product of the same file on disk.
> >>
> >
> > But they are not the same. When re-loading the device path (as you
> > suggest below) you will get a completely different file, and the only
> > way to get at the payload is to execute it.
> >
> > So using the same device path is out of the question imo.
>
> How about appending a VenHW() node with a decompressor specific GUID at
> the end of the DP?
>
> I think that is the most UEFIish way to express that the handle is
> derived from the compressed file.
>
> You could even put additional information into the VenHW() node like the
> compression type or the compressed size.
>

Uhm, yes, that is what I am proposing further down in this email.

See below.

> >
> >>>
> >>> What we could do is define a vendor GUID for the decompressed kernel,
> >>> and create a device path for it. That way, you can grab the
> >>> loaded_image of the parent to obtain this information.
> >>>
> >>> What did you have in mind as a use case?
> >>
> >> The device-path could be used in the kernel log.
> >>
> >> It can be used to find the device or folder with initrd where we use
> >> initrd= on the command line.
> >>
> >> You could use the device path to access the original file, e.g. to read
> >> additional information.
> >>
> >> For all use cases you would want to have the original device path.
> >>
> >
> > What we could do is:
> >
> > - define a device path in the decompressor, e.g.,
> >
> > <original device path>/Offset(<start>, <end>)/VendorMedia(xxx-xxx-xxx,
> > <compression type>)
> >
> > where start, end and compression type describe the compressed payload
> > inside the decompressor executable. (The compression type could be
> > omitted, or could be a separate node.)
> >
> > - install the LoadFile2 protocol and the device path protocol onto a
> > handle, and move the decompression logic into the LoadFile2
> > implementation
> >
> > - drop the SourceBuffer and SourceSize arguments to LoadImage(), and
> > pass the device path instead, so that LoadFile2 will be invoked by
> > LoadImage directly to perform the decompression.
> >
> > That way, we retain the information about the outer file, and each
> > piece is described in detail in device path notation. As a bonus, we
> > could easily expose the compressed part separately, if there is a need
> > for that.
> >
> > This doesn't cover the initrd= issue you raised, but that is something
> > we could address later in the stub if we wanted to (but I don't think
> > initrd= is something we should care too much about)
>

Re: [PATCH v3 4/6] efi/libstub: implement generic EFI zboot

[Heinrich Schuchardt]

On 8/19/22 09:07, Ard Biesheuvel wrote:
> On Fri, 19 Aug 2022 at 09:01, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> On 8/19/22 08:52, Ard Biesheuvel wrote:
>>> On Fri, 19 Aug 2022 at 07:29, Heinrich Schuchardt
>>> <heinrich.schuchardt@canonical.com> wrote:
>>>>
>>>>
>>>>
>>>> On 8/18/22 19:10, Ard Biesheuvel wrote:
>>>>> On Thu, 18 Aug 2022 at 18:42, Heinrich Schuchardt
>>>>> <heinrich.schuchardt@canonical.com> wrote:
>>>>>>
>>>>>> On 8/17/22 13:03, Ard Biesheuvel wrote:
>>>>>>> Implement a minimal EFI app that decompresses the real kernel image and
>>>>>>> launches it using the firmware's LoadImage and StartImage boot services.
>>>>>>> This removes the need for any arch-specific hacks.
>>>>>>>
>>>>>>> Note that on systems that have UEFI secure boot policies enabled,
>>>>>>> LoadImage/StartImage require images to be signed, or their hashes known
>>>>>>> a priori, in order to be permitted to boot.
>>>>>>>
>>>>>>> There are various possible strategies to work around this requirement,
>>>>>>> but they all rely either on overriding internal PI/DXE protocols (which
>>>>>>> are not part of the EFI spec) or omitting the firmware provided
>>>>>>> LoadImage() and StartImage() boot services, which is also undesirable,
>>>>>>> given that they encapsulate platform specific policies related to secure
>>>>>>> boot and measured boot, but also related to memory permissions (whether
>>>>>>> or not and which types of heap allocations have both write and execute
>>>>>>> permissions.)
>>>>>>>
>>>>>>> The only generic and truly portable way around this is to simply sign
>>>>>>> both the inner and the outer image with the same key/cert pair, so this
>>>>>>> is what is implemented here.
>>>>>>>
>>>>>>> BZIP2 has been omitted from the set of supported compression algorithms,
>>>>>>> given that its performance is mediocre both in speed and size, and it
>>>>>>> uses a disproportionate amount of memory. For optimal compression, use
>>>>>>> LZMA. For the fastest boot speed, use LZO.
>>>>>>>
>>>>>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>>>>>> ---
>>>>>>>      drivers/firmware/efi/Kconfig                |  31 ++++-
>>>>>>>      drivers/firmware/efi/libstub/Makefile       |   8 +-
>>>>>>>      drivers/firmware/efi/libstub/Makefile.zboot |  69 ++++++++++
>>>>>>>      drivers/firmware/efi/libstub/zboot-header.S | 139 ++++++++++++++++++++
>>>>>>>      drivers/firmware/efi/libstub/zboot.c        | 101 ++++++++++++++
>>>>>>>      drivers/firmware/efi/libstub/zboot.lds      |  39 ++++++
>>>>>>>      6 files changed, 382 insertions(+), 5 deletions(-)
>>>>>>>
>>>>> ...
>>>>>>> diff --git a/drivers/firmware/efi/libstub/zboot.c b/drivers/firmware/efi/libstub/zboot.c
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..9cf968e90775
>>>>>>> --- /dev/null
>>>>>>> +++ b/drivers/firmware/efi/libstub/zboot.c
>>>>> ...
>>>>>>> +efi_status_t __efiapi efi_zboot_entry(efi_handle_t handle,
>>>>>>> +                                   efi_system_table_t *systab)
>>>>>>> +{
>>>>>>> +     static efi_guid_t loaded_image = LOADED_IMAGE_PROTOCOL_GUID;
>>>>>>> +     efi_loaded_image_t *parent, *child;
>>>>>>> +     unsigned long image_buffer;
>>>>>>> +     efi_handle_t child_handle;
>>>>>>> +     efi_status_t status;
>>>>>>> +     int ret;
>>>>>>> +
>>>>>>> +     WRITE_ONCE(efi_system_table, systab);
>>>>>>> +
>>>>>>> +     free_mem_ptr = (unsigned long)&zboot_heap;
>>>>>>> +     free_mem_end_ptr = free_mem_ptr + sizeof(zboot_heap);
>>>>>>> +
>>>>>>> +     status = efi_bs_call(handle_protocol, handle, &loaded_image,
>>>>>>> +                          (void **)&parent);
>>>>>>> +     if (status != EFI_SUCCESS) {
>>>>>>> +             log(L"Failed to locate parent's loaded image protocol\n");
>>>>>>> +             return status;
>>>>>>> +     }
>>>>>>> +
>>>>>>> +     status = efi_allocate_pages(uncompressed_size, &image_buffer, ULONG_MAX);
>>>>>>> +     if (status != EFI_SUCCESS) {
>>>>>>> +             log(L"Failed to allocate memory\n");
>>>>>>> +             return status;
>>>>>>> +     }
>>>>>>> +
>>>>>>> +     ret = __decompress(_gzdata_start, _gzdata_end - _gzdata_start, NULL,
>>>>>>> +                        NULL, (unsigned char *)image_buffer, 0, NULL,
>>>>>>> +                        error);
>>>>>>> +     if (ret < 0) {
>>>>>>> +             log(L"Decompression failed\n");
>>>>>>> +             return EFI_LOAD_ERROR;
>>>>>>> +     }
>>>>>>> +
>>>>>>> +     status = efi_bs_call(load_image, false, handle, NULL,
>>>>>>
>>>>>> I would prefer to pass the device path of the compressed image instead
>>>>>> of NULL. This way information is not lost.
>>>>>>
>>>>>
>>>>> That way, we will have two loaded images with different handles
>>>>> claiming to be loaded from the same device path - I don't think that
>>>>> is appropriate tbh.
>>>>
>>>> They both are the product of the same file on disk.
>>>>
>>>
>>> But they are not the same. When re-loading the device path (as you
>>> suggest below) you will get a completely different file, and the only
>>> way to get at the payload is to execute it.
>>>
>>> So using the same device path is out of the question imo.
>>
>> How about appending a VenHW() node with a decompressor specific GUID at
>> the end of the DP?
>>
>> I think that is the most UEFIish way to express that the handle is
>> derived from the compressed file.
>>
>> You could even put additional information into the VenHW() node like the
>> compression type or the compressed size.
>>
> 
> Uhm, yes, that is what I am proposing further down in this email.
> 
> See below.
> 
>>>
>>>>>
>>>>> What we could do is define a vendor GUID for the decompressed kernel,
>>>>> and create a device path for it. That way, you can grab the
>>>>> loaded_image of the parent to obtain this information.
>>>>>
>>>>> What did you have in mind as a use case?
>>>>
>>>> The device-path could be used in the kernel log.
>>>>
>>>> It can be used to find the device or folder with initrd where we use
>>>> initrd= on the command line.
>>>>
>>>> You could use the device path to access the original file, e.g. to read
>>>> additional information.
>>>>
>>>> For all use cases you would want to have the original device path.
>>>>
>>>
>>> What we could do is:
>>>
>>> - define a device path in the decompressor, e.g.,
>>>
>>> <original device path>/Offset(<start>, <end>)/VendorMedia(xxx-xxx-xxx,
>>> <compression type>)
>>>
>>> where start, end and compression type describe the compressed payload
>>> inside the decompressor executable. (The compression type could be
>>> omitted, or could be a separate node.)
>>>
>>> - install the LoadFile2 protocol and the device path protocol onto a
>>> handle, and move the decompression logic into the LoadFile2
>>> implementation

Why would you create a LoadFile2 implementation if the decompressor is 
the only user? In this case I think an internal function call is more 
adequate.

A LoadFile2 protocol would make sense if the compressed image contains 
both a compressed kernel and the initrd and you wanted to provide the 
initrd via the LoadFile2 protocol.

For Iot use cases it would make sense to have a standalone process which 
you can use to:

* compress a kernel
* create a binary containing
* * a prepended decompressor binary
* * the compressed kernel
* * an optional initrd
* * an optional device-tree
* sign the complete file

At runtime the decompressor would:

* decompress the kernel
* create a LoadFile2 protocol for the initrd
* call the firmware to fix-up the device-tree
* install the fixed-up device-tree
* invoke the EFI stub of the kernel

This way we could have one binary where all relevant components are 
inside a single signed image.

Best regards

Heinrich

>>>
>>> - drop the SourceBuffer and SourceSize arguments to LoadImage(), and
>>> pass the device path instead, so that LoadFile2 will be invoked by
>>> LoadImage directly to perform the decompression.
>>>
>>> That way, we retain the information about the outer file, and each
>>> piece is described in detail in device path notation. As a bonus, we
>>> could easily expose the compressed part separately, if there is a need
>>> for that.
>>>
>>> This doesn't cover the initrd= issue you raised, but that is something
>>> we could address later in the stub if we wanted to (but I don't think
>>> initrd= is something we should care too much about)
>>

Re: [PATCH v3 4/6] efi/libstub: implement generic EFI zboot

[Ard Biesheuvel]

On Fri, 19 Aug 2022 at 09:41, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> On 8/19/22 09:07, Ard Biesheuvel wrote:
> > On Fri, 19 Aug 2022 at 09:01, Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> >>
> >> On 8/19/22 08:52, Ard Biesheuvel wrote:
> >>> On Fri, 19 Aug 2022 at 07:29, Heinrich Schuchardt
> >>> <heinrich.schuchardt@canonical.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 8/18/22 19:10, Ard Biesheuvel wrote:
...
> >>>>> What we could do is define a vendor GUID for the decompressed kernel,
> >>>>> and create a device path for it. That way, you can grab the
> >>>>> loaded_image of the parent to obtain this information.
> >>>>>
> >>>>> What did you have in mind as a use case?
> >>>>
> >>>> The device-path could be used in the kernel log.
> >>>>
> >>>> It can be used to find the device or folder with initrd where we use
> >>>> initrd= on the command line.
> >>>>
> >>>> You could use the device path to access the original file, e.g. to read
> >>>> additional information.
> >>>>
> >>>> For all use cases you would want to have the original device path.
> >>>>
> >>>
> >>> What we could do is:
> >>>
> >>> - define a device path in the decompressor, e.g.,
> >>>
> >>> <original device path>/Offset(<start>, <end>)/VendorMedia(xxx-xxx-xxx,
> >>> <compression type>)
> >>>
> >>> where start, end and compression type describe the compressed payload
> >>> inside the decompressor executable. (The compression type could be
> >>> omitted, or could be a separate node.)
> >>>
> >>> - install the LoadFile2 protocol and the device path protocol onto a
> >>> handle, and move the decompression logic into the LoadFile2
> >>> implementation
>
> Why would you create a LoadFile2 implementation if the decompressor is
> the only user? In this case I think an internal function call is more
> adequate.
>

You argued that you would want to access the original file, no? When
using the SourceBuffer argument to LoadImage(), the original data is
gone. When you pass a LoadFile2 protocol, you can use the device path
to reload the data (compressed or uncompressed) as needed.

> A LoadFile2 protocol would make sense if the compressed image contains
> both a compressed kernel and the initrd and you wanted to provide the
> initrd via the LoadFile2 protocol.
>

The initrd loading is a completely separate issue, and the fact that
we might use LoadFile2 for both is just a coincidence.

> For Iot use cases it would make sense to have a standalone process which
> you can use to:
>
> * compress a kernel
> * create a binary containing
> * * a prepended decompressor binary
> * * the compressed kernel
> * * an optional initrd
> * * an optional device-tree
> * sign the complete file
>
> At runtime the decompressor would:
>
> * decompress the kernel
> * create a LoadFile2 protocol for the initrd
> * call the firmware to fix-up the device-tree
> * install the fixed-up device-tree
> * invoke the EFI stub of the kernel
>
> This way we could have one binary where all relevant components are
> inside a single signed image.
>

The only generic and portable way to move data into memory and execute
it as code is using LoadImage/StartImage. Everything else is a hack,
and hacks are notoriously non-portable between architectures and
firmware implementations.

This means the 'inner' executable *must* be invoked using
LoadImage/StartImage, which implies it must be signed if secure boot
is enabled.

Fixing up the device tree is tied to specific architectures, and the
notion that the OS should be involved in poking the firmware at the
right time to fix it up is also highly arch dependent. Which means it
does not belong in a generic EFI decompressor either.

What you are describing is essentially the systemd-stub, which also
omits LoadImage/StartImage, which means it has to parse the PE/COFF
image, load the sections, apply the relocations, etc etc, all of which
it surely got correct right off the bat ...

The reason I am so adamant about this is that we are digging ourselves
into a hole here, and the more instances we add of this pattern, the
more difficult it is to reason about it, and to fix it when things
break.

Shim should not need to exist, but we all know it has to. But ideally,
it would only hook LoadImage/StartImage to interpose its own
implementations, leaving subsequent stages none the wiser. I
personally find it very disappointing that distro GRUB cannot work
without shim even if the distro's signing key is in db - this is a
missed opportunity imo.

But if we repeat this pattern at every level above it, (i.e., open
code PE app launch to avoid using LoadImage/StartImage), reasoning
about secure boot and measured boot becomes very difficult, and every
stage needs to invoke the TCG protocols directly etc etc

[PATCH v3 5/6] arm64: efi: enable generic EFI compressed boot

[Ard Biesheuvel]

Wire up the generic EFI zboot support for arm64.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/Makefile      | 7 ++++++-
 arch/arm64/boot/Makefile | 6 ++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 6d9d4a58b898..6a8b81cfa648 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -153,7 +153,7 @@ libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
 boot		:= arch/arm64/boot
 KBUILD_IMAGE	:= $(boot)/Image.gz
 
-all:	Image.gz
+all:	$(notdir $(KBUILD_IMAGE))
 
 
 Image: vmlinux
@@ -162,6 +162,11 @@ Image: vmlinux
 Image.%: Image
 	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
 
+ifneq ($(CONFIG_EFI_ZBOOT),)
+zImage.efi: Image
+	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
+endif
+
 install: KBUILD_IMAGE := $(boot)/Image
 install zinstall:
 	$(call cmd,install)
diff --git a/arch/arm64/boot/Makefile b/arch/arm64/boot/Makefile
index a0e3dedd2883..3e16bb85cdad 100644
--- a/arch/arm64/boot/Makefile
+++ b/arch/arm64/boot/Makefile
@@ -38,3 +38,9 @@ $(obj)/Image.lzo: $(obj)/Image FORCE
 
 $(obj)/Image.zst: $(obj)/Image FORCE
 	$(call if_changed,zstd)
+
+EFI_ZBOOT_PAYLOAD	:= $(obj)/Image
+EFI_ZBOOT_BFD_TARGET	:= elf64-littleaarch64
+EFI_ZBOOT_MACH_TYPE	:= ARM64
+
+include $(srctree)/drivers/firmware/efi/libstub/Makefile.zboot
-- 
2.35.1

[PATCH v3 6/6] riscv: efi: enable generic EFI compressed boot

[Ard Biesheuvel]

Wire up the generic EFI zboot support for RISC-V.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/riscv/Makefile      | 5 +++++
 arch/riscv/boot/Makefile | 6 ++++++
 2 files changed, 11 insertions(+)

diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 3fa8ef336822..23f1b934c825 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -150,6 +150,11 @@ $(BOOT_TARGETS): vmlinux
 Image.%: Image
 	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
 
+ifneq ($(CONFIG_EFI_ZBOOT),)
+zImage.efi: Image
+	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
+endif
+
 install: KBUILD_IMAGE := $(boot)/Image
 zinstall: KBUILD_IMAGE := $(boot)/Image.gz
 install zinstall:
diff --git a/arch/riscv/boot/Makefile b/arch/riscv/boot/Makefile
index becd0621071c..82970c4a91b5 100644
--- a/arch/riscv/boot/Makefile
+++ b/arch/riscv/boot/Makefile
@@ -58,3 +58,9 @@ $(obj)/Image.lzo: $(obj)/Image FORCE
 
 $(obj)/loader.bin: $(obj)/loader FORCE
 	$(call if_changed,objcopy)
+
+EFI_ZBOOT_PAYLOAD	:= $(obj)/Image
+EFI_ZBOOT_BFD_TARGET	:= elf$(BITS)-littleriscv
+EFI_ZBOOT_MACH_TYPE	:= RISCV$(BITS)
+
+include $(srctree)/drivers/firmware/efi/libstub/Makefile.zboot
-- 
2.35.1