[PATCHv2 0/3] kexec: Add support for UKI format kernel

[PATCHv2 0/3] kexec: Add support for UKI format kernel

[Pingfan Liu]

As a UEFI PE format kernel image becomes more popular, there is a need
for kexec to reboot those kinds of images.

After the introduction of the UKI (another PE), at present, there are
three competitive methods to support that goal, but all of them have
pros and cons.[1] It seems that none of them can be accepted in the near
future.  Therefore, we are resorting to the user space kexec-tools to
parse the UKI format for the time being.

By parsing the UKI, systemd-stub is stepped around and PCM will not affect
the boot up of the second system.

[1]: https://github.com/rhkdump/kexec_uefi/blob/main/overview.md#the-competitive-solutions

Cc: Simon Horman <horms@kernel.org>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Baoquan He <bhe@redhat.com>
Cc: Dave Young <dyoung@redhat.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Jan Hendrik Farr <kernel@jfarr.cc>
Cc: Philipp Rudo <prudo@redhat.com>
Cc: Lennart Poettering <mzxreary@0pointer.de>
Cc: kexec@lists.infradead.org
---
v1 -> v2:
Add include/pe.h in makefile to fix 'make dist'
Rename default_initrd_fd to implicit_initrd_fd


Pingfan Liu (3):
  kexec: Introduce implicit_initrd_fd to pass internal initrd
    information
  kexec: Introduce UKI image parser
  arm64: Support UKI image format

 include/Makefile                     |   1 +
 include/pe.h                         | 104 +++++++++++++++++++++++
 kexec/Makefile                       |   1 +
 kexec/arch/arm64/kexec-arm64.c       |   1 +
 kexec/arch/arm64/kexec-image-arm64.c |   3 +-
 kexec/arch/x86_64/kexec-bzImage64.c  |   3 +-
 kexec/kexec-uki.c                    | 122 +++++++++++++++++++++++++++
 kexec/kexec.c                        |   2 +
 kexec/kexec.h                        |   5 ++
 9 files changed, 240 insertions(+), 2 deletions(-)
 create mode 100644 include/pe.h
 create mode 100644 kexec/kexec-uki.c

-- 
2.41.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCHv2 1/3] kexec: Introduce implicit_initrd_fd to pass internal initrd information

[Pingfan Liu]

At present, initrd information is passed through --initrd args, and
handled until image load. But for the UKI case, the initrd may be stored
implicitly in the .initrd section.

It means that initrd is perceived at the probe stage. And there should
be a way to carry this information forward to the load stage. In oder to
implement that, introducing a global implicit_initrd_fd.

Signed-off-by: Pingfan Liu <piliu@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: kexec@lists.infradead.org
---
 kexec/arch/arm64/kexec-image-arm64.c | 3 ++-
 kexec/arch/x86_64/kexec-bzImage64.c  | 3 ++-
 kexec/kexec.c                        | 2 ++
 kexec/kexec.h                        | 1 +
 4 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/kexec/arch/arm64/kexec-image-arm64.c b/kexec/arch/arm64/kexec-image-arm64.c
index a196747..b6f8912 100644
--- a/kexec/arch/arm64/kexec-image-arm64.c
+++ b/kexec/arch/arm64/kexec-image-arm64.c
@@ -50,7 +50,8 @@ int image_arm64_load(int argc, char **argv, const char *kernel_buf,
 				result = EFAILED;
 				goto exit;
 			}
-		}
+		} else if (implicit_initrd_fd != -1)
+			info->initrd_fd = implicit_initrd_fd;
 
 		if (arm64_opts.command_line) {
 			info->command_line = (char *)arm64_opts.command_line;
diff --git a/kexec/arch/x86_64/kexec-bzImage64.c b/kexec/arch/x86_64/kexec-bzImage64.c
index aba4e3b..210bc0a 100644
--- a/kexec/arch/x86_64/kexec-bzImage64.c
+++ b/kexec/arch/x86_64/kexec-bzImage64.c
@@ -307,7 +307,8 @@ int bzImage64_load_file(int argc, char **argv, struct kexec_info *info)
 			ret = -1;
 			goto out;
 		}
-	}
+	} else if (implicit_initrd_fd != -1)
+		info->initrd_fd = implicit_initrd_fd;
 
 	info->command_line = command_line;
 	info->command_line_len = command_line_len;
diff --git a/kexec/kexec.c b/kexec/kexec.c
index 7c614b0..4f51987 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -67,6 +67,8 @@ int do_hotplug = 0;
 static unsigned long kexec_flags = 0;
 /* Flags for kexec file (fd) based syscall */
 static unsigned long kexec_file_flags = 0;
+/* initrd detected in probe phase */
+int implicit_initrd_fd = -1;
 int kexec_debug = 0;
 
 void dbgprint_mem_range(const char *prefix, struct memory_range *mr, int nr_mr)
diff --git a/kexec/kexec.h b/kexec/kexec.h
index 31c323f..e70c18d 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -114,6 +114,7 @@ do { \
 #define _ALIGN(addr, size)     _ALIGN_UP(addr, size)
 
 extern unsigned long long mem_min, mem_max;
+extern int implicit_initrd_fd;
 extern int kexec_debug;
 
 #define dbgprintf(...) \
-- 
2.41.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCHv2 2/3] kexec: Introduce UKI image parser

[Pingfan Liu]

A UKI image is a PE file that consists of several sections, typically
including: .text, .data, .linux, .initrd, .cmdline, and others.

The kernel image is stored in the .linux section, which is one of the
formats currently recognized by kexec-tools. Therefore, the UKI parser
can be used to strip away the UKI layer, allowing the other parser to
continue the procession of the kernel image.

Signed-off-by: Pingfan Liu <piliu@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: kexec@lists.infradead.org
---
 include/Makefile  |   1 +
 include/pe.h      | 104 +++++++++++++++++++++++++++++++++++++++
 kexec/Makefile    |   1 +
 kexec/kexec-uki.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++
 kexec/kexec.h     |   4 ++
 5 files changed, 232 insertions(+)
 create mode 100644 include/pe.h
 create mode 100644 kexec/kexec-uki.c

diff --git a/include/Makefile b/include/Makefile
index cd88a26..6a3e854 100644
--- a/include/Makefile
+++ b/include/Makefile
@@ -8,6 +8,7 @@ dist += include/Makefile		\
 	include/x86/mb_header.h		\
 	include/x86/multiboot2.h	\
 	include/elf.h			\
+	include/pe.h			\
 	include/image.h			\
 	include/unused.h		\
 	include/boot/linuxbios_tables.h	\
diff --git a/include/pe.h b/include/pe.h
new file mode 100644
index 0000000..2617074
--- /dev/null
+++ b/include/pe.h
@@ -0,0 +1,104 @@
+/*
+ * Extract from linux kernel include/linux/pe.h
+ */
+
+#ifndef __PE_H__
+#define __PE_H__
+
+struct pe_hdr {
+	uint32_t magic;		/* PE magic */
+	uint16_t machine;	/* machine type */
+	uint16_t sections;	/* number of sections */
+	uint32_t timestamp;	/* time_t */
+	uint32_t symbol_table;	/* symbol table offset */
+	uint32_t symbols;	/* number of symbols */
+	uint16_t opt_hdr_size;	/* size of optional header */
+	uint16_t flags;		/* flags */
+};
+
+/* the fact that pe32 isn't padded where pe32+ is 64-bit means union won't
+ * work right.  vomit. */
+struct pe32_opt_hdr {
+	/* "standard" header */
+	uint16_t magic;		/* file type */
+	uint8_t  ld_major;	/* linker major version */
+	uint8_t  ld_minor;	/* linker minor version */
+	uint32_t text_size;	/* size of text section(s) */
+	uint32_t data_size;	/* size of data section(s) */
+	uint32_t bss_size;	/* size of bss section(s) */
+	uint32_t entry_point;	/* file offset of entry point */
+	uint32_t code_base;	/* relative code addr in ram */
+	uint32_t data_base;	/* relative data addr in ram */
+	/* "windows" header */
+	uint32_t image_base;	/* preferred load address */
+	uint32_t section_align;	/* alignment in bytes */
+	uint32_t file_align;	/* file alignment in bytes */
+	uint16_t os_major;	/* major OS version */
+	uint16_t os_minor;	/* minor OS version */
+	uint16_t image_major;	/* major image version */
+	uint16_t image_minor;	/* minor image version */
+	uint16_t subsys_major;	/* major subsystem version */
+	uint16_t subsys_minor;	/* minor subsystem version */
+	uint32_t win32_version;	/* reserved, must be 0 */
+	uint32_t image_size;	/* image size */
+	uint32_t header_size;	/* header size rounded up to
+				   file_align */
+	uint32_t csum;		/* checksum */
+	uint16_t subsys;	/* subsystem */
+	uint16_t dll_flags;	/* more flags! */
+	uint32_t stack_size_req;/* amt of stack requested */
+	uint32_t stack_size;	/* amt of stack required */
+	uint32_t heap_size_req;	/* amt of heap requested */
+	uint32_t heap_size;	/* amt of heap required */
+	uint32_t loader_flags;	/* reserved, must be 0 */
+	uint32_t data_dirs;	/* number of data dir entries */
+};
+
+struct pe32plus_opt_hdr {
+	uint16_t magic;		/* file type */
+	uint8_t  ld_major;	/* linker major version */
+	uint8_t  ld_minor;	/* linker minor version */
+	uint32_t text_size;	/* size of text section(s) */
+	uint32_t data_size;	/* size of data section(s) */
+	uint32_t bss_size;	/* size of bss section(s) */
+	uint32_t entry_point;	/* file offset of entry point */
+	uint32_t code_base;	/* relative code addr in ram */
+	/* "windows" header */
+	uint64_t image_base;	/* preferred load address */
+	uint32_t section_align;	/* alignment in bytes */
+	uint32_t file_align;	/* file alignment in bytes */
+	uint16_t os_major;	/* major OS version */
+	uint16_t os_minor;	/* minor OS version */
+	uint16_t image_major;	/* major image version */
+	uint16_t image_minor;	/* minor image version */
+	uint16_t subsys_major;	/* major subsystem version */
+	uint16_t subsys_minor;	/* minor subsystem version */
+	uint32_t win32_version;	/* reserved, must be 0 */
+	uint32_t image_size;	/* image size */
+	uint32_t header_size;	/* header size rounded up to
+				   file_align */
+	uint32_t csum;		/* checksum */
+	uint16_t subsys;	/* subsystem */
+	uint16_t dll_flags;	/* more flags! */
+	uint64_t stack_size_req;/* amt of stack requested */
+	uint64_t stack_size;	/* amt of stack required */
+	uint64_t heap_size_req;	/* amt of heap requested */
+	uint64_t heap_size;	/* amt of heap required */
+	uint32_t loader_flags;	/* reserved, must be 0 */
+	uint32_t data_dirs;	/* number of data dir entries */
+};
+
+struct section_header {
+	char name[8];			/* name or "/12\0" string tbl offset */
+	uint32_t virtual_size;		/* size of loaded section in ram */
+	uint32_t virtual_address;	/* relative virtual address */
+	uint32_t raw_data_size;		/* size of the section */
+	uint32_t data_addr;		/* file pointer to first page of sec */
+	uint32_t relocs;		/* file pointer to relocation entries */
+	uint32_t line_numbers;		/* line numbers! */
+	uint16_t num_relocs;		/* number of relocations */
+	uint16_t num_lin_numbers;	/* srsly. */
+	uint32_t flags;
+};
+
+#endif
diff --git a/kexec/Makefile b/kexec/Makefile
index 11682bf..d4f26d7 100644
--- a/kexec/Makefile
+++ b/kexec/Makefile
@@ -18,6 +18,7 @@ KEXEC_SRCS_base += kexec/kexec-elf-core.c
 KEXEC_SRCS_base += kexec/kexec-elf-rel.c
 KEXEC_SRCS_base += kexec/kexec-elf-boot.c
 KEXEC_SRCS_base += kexec/kexec-pe-zboot.c
+KEXEC_SRCS_base += kexec/kexec-uki.c
 KEXEC_SRCS_base += kexec/kexec-iomem.c
 KEXEC_SRCS_base += kexec/firmware_memmap.c
 KEXEC_SRCS_base += kexec/crashdump.c
diff --git a/kexec/kexec-uki.c b/kexec/kexec-uki.c
new file mode 100644
index 0000000..202fd89
--- /dev/null
+++ b/kexec/kexec-uki.c
@@ -0,0 +1,122 @@
+#include <limits.h>
+#include <stdint.h>
+#include <string.h>
+#include <stdio.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <pe.h>
+#include "kexec.h"
+
+#define UKI_LINUX_SECTION ".linux"
+#define UKI_INITRD_SECTION ".initrd"
+#define UKI_CMDLINE_SECTION ".cmdline"
+#define UKI_DTB_SECTION ".dtb"
+
+#define FILENAME_UKI_INITRD          "/tmp/InitrdXXXXXX"
+
+static int embeded_linux_format_index = -1;
+
+/*
+ * Return -1 if not PE, else offset of the PE header
+ */
+static int get_pehdr_offset(const char *buf)
+{
+	int pe_hdr_offset;
+
+	pe_hdr_offset = *((int *)(buf + 0x3c));
+	buf += pe_hdr_offset;
+	if (!!memcmp(buf, "PE\0\0", 4)) {
+		printf("Not a PE file\n");
+		return -1;
+	}
+
+	return pe_hdr_offset;
+}
+
+int uki_image_probe(const char *file_buf, off_t buf_sz)
+{
+	struct pe_hdr *pe_hdr;
+	struct pe32plus_opt_hdr *opt_hdr;
+	struct section_header *sect_hdr;
+	int pe_hdr_offset, section_nr, linux_sz = -1;
+	char *pe_part_buf, *linux_src;
+	char *initrd_fname = NULL;
+	int initrd_fd = -1;
+
+	pe_hdr_offset = get_pehdr_offset(file_buf);
+	pe_part_buf = (char *)file_buf + pe_hdr_offset;
+	pe_hdr = (struct pe_hdr *)pe_part_buf;
+	if (pe_hdr->opt_hdr_size == 0) {
+		printf("ERR: optional header is missing\n");
+		return -1;
+	}
+	section_nr = pe_hdr->sections;
+	opt_hdr = (struct pe32plus_opt_hdr *)(pe_part_buf + sizeof(struct pe_hdr));
+	sect_hdr = (struct section_header *)((char *)opt_hdr + pe_hdr->opt_hdr_size);
+
+	for (int i = 0; i < section_nr; i++) {
+		if (!strcmp(sect_hdr->name, UKI_LINUX_SECTION)) {
+			/* data_addr is relative to the whole file */
+			linux_src = (char *)file_buf + sect_hdr->data_addr;
+			linux_sz = sect_hdr->raw_data_size;
+
+		} else if (!strcmp(sect_hdr->name, UKI_INITRD_SECTION)) {
+			if (!(initrd_fname = strdup(FILENAME_UKI_INITRD))) {
+				dbgprintf("%s: Can't duplicate strings\n", __func__);
+				goto next;
+			}
+
+			if ((initrd_fd = mkstemp(initrd_fname)) < 0) {
+				dbgprintf("%s: Can't open file %s\n", __func__,	initrd_fname);
+				goto next;
+			}
+
+			if (write(initrd_fd, (char *)file_buf + sect_hdr->data_addr,
+					sect_hdr->raw_data_size) != sect_hdr->raw_data_size) {
+				dbgprintf("%s: Can't write the compressed file %s\n",
+						__func__, initrd_fname);
+				goto next;
+			} else {
+				implicit_initrd_fd = open(initrd_fname, O_RDONLY);
+				close(initrd_fd);
+			}
+		}
+next:
+		sect_hdr++;
+	}
+
+	if (linux_sz == -1) {
+		printf("ERR: can not find .linux section\n");
+		return -1;
+	}
+	/*
+	 * After stripping the UKI coat, the real kernel format can be handled now.
+	 */
+	for (int i = 0; i < file_types; i++) {
+		/* kernel_fd will be created by probe */
+		if (file_type[i].probe != uki_image_probe &&
+		    file_type[i].probe(linux_src, linux_sz) >= 0) {
+			embeded_linux_format_index = i;
+			break;
+		}
+	}
+	if (embeded_linux_format_index < 0) {
+		printf("Can not recognize the kernel format in .linux section\n");
+		return -1;
+	}
+	return 0;
+}
+
+int uki_image_load(int argc, char **argv, const char *buf, off_t len,
+			struct kexec_info *info)
+{
+	return file_type[embeded_linux_format_index].load(argc, argv, buf, len, info);
+}
+
+void uki_image_usage(void)
+{
+	printf(
+"     An UKI image.\n");
+}
diff --git a/kexec/kexec.h b/kexec/kexec.h
index e70c18d..a2e19c4 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -357,4 +357,8 @@ static inline void ultoa(unsigned long val, char *str)
 	str[pos] = 0;
 }
 
+extern int uki_image_probe(const char *file_buf, off_t buf_sz);
+extern int uki_image_load(int argc, char **argv, const char *buf, off_t len,
+		struct kexec_info *info);
+extern void uki_image_usage(void);
 #endif /* KEXEC_H */
-- 
2.41.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCHv2 3/3] arm64: Support UKI image format

[Pingfan Liu]

Signed-off-by: Pingfan Liu <piliu@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: kexec@lists.infradead.org
---
 kexec/arch/arm64/kexec-arm64.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
index 55e822b..9091f40 100644
--- a/kexec/arch/arm64/kexec-arm64.c
+++ b/kexec/arch/arm64/kexec-arm64.c
@@ -75,6 +75,7 @@ struct file_type file_type[] = {
 	{"Image", image_arm64_probe, image_arm64_load, image_arm64_usage},
 	{"uImage", uImage_arm64_probe, uImage_arm64_load, uImage_arm64_usage},
 	{"vmlinuz", pez_arm64_probe, pez_arm64_load, pez_arm64_usage},
+	{"uki", uki_image_probe, uki_image_load, uki_image_usage},
 };
 
 int file_types = sizeof(file_type) / sizeof(file_type[0]);
-- 
2.41.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCHv2 2/3] kexec: Introduce UKI image parser

[Simon Horman]

On Wed, Oct 16, 2024 at 07:34:14PM +0800, Pingfan Liu wrote:
> A UKI image is a PE file that consists of several sections, typically
> including: .text, .data, .linux, .initrd, .cmdline, and others.
> 
> The kernel image is stored in the .linux section, which is one of the
> formats currently recognized by kexec-tools. Therefore, the UKI parser
> can be used to strip away the UKI layer, allowing the other parser to
> continue the procession of the kernel image.
> 
> Signed-off-by: Pingfan Liu <piliu@redhat.com>

...

> diff --git a/kexec/kexec-uki.c b/kexec/kexec-uki.c

...

> +int uki_image_probe(const char *file_buf, off_t buf_sz)
> +{
> +	struct pe_hdr *pe_hdr;
> +	struct pe32plus_opt_hdr *opt_hdr;
> +	struct section_header *sect_hdr;
> +	int pe_hdr_offset, section_nr, linux_sz = -1;
> +	char *pe_part_buf, *linux_src;
> +	char *initrd_fname = NULL;
> +	int initrd_fd = -1;
> +
> +	pe_hdr_offset = get_pehdr_offset(file_buf);
> +	pe_part_buf = (char *)file_buf + pe_hdr_offset;
> +	pe_hdr = (struct pe_hdr *)pe_part_buf;
> +	if (pe_hdr->opt_hdr_size == 0) {
> +		printf("ERR: optional header is missing\n");
> +		return -1;
> +	}
> +	section_nr = pe_hdr->sections;
> +	opt_hdr = (struct pe32plus_opt_hdr *)(pe_part_buf + sizeof(struct pe_hdr));
> +	sect_hdr = (struct section_header *)((char *)opt_hdr + pe_hdr->opt_hdr_size);
> +
> +	for (int i = 0; i < section_nr; i++) {
> +		if (!strcmp(sect_hdr->name, UKI_LINUX_SECTION)) {
> +			/* data_addr is relative to the whole file */
> +			linux_src = (char *)file_buf + sect_hdr->data_addr;
> +			linux_sz = sect_hdr->raw_data_size;
> +
> +		} else if (!strcmp(sect_hdr->name, UKI_INITRD_SECTION)) {
> +			if (!(initrd_fname = strdup(FILENAME_UKI_INITRD))) {
> +				dbgprintf("%s: Can't duplicate strings\n", __func__);
> +				goto next;
> +			}
> +
> +			if ((initrd_fd = mkstemp(initrd_fname)) < 0) {
> +				dbgprintf("%s: Can't open file %s\n", __func__,	initrd_fname);
> +				goto next;
> +			}
> +
> +			if (write(initrd_fd, (char *)file_buf + sect_hdr->data_addr,
> +					sect_hdr->raw_data_size) != sect_hdr->raw_data_size) {
> +				dbgprintf("%s: Can't write the compressed file %s\n",
> +						__func__, initrd_fname);
> +				goto next;

I plan to apply this series, but initrd_fd appears to be leaked here.
Could you send a follow-up patch to address that? Perhaps like this:

			err = write(...);
			close(initrd_fd)
			if (err != sect_hdr->raw_data_size) {
				...
			} else {
				...
			}

Thanks!

> +			} else {
> +				implicit_initrd_fd = open(initrd_fname, O_RDONLY);
> +				close(initrd_fd);
> +			}
> +		}
> +next:
> +		sect_hdr++;
> +	}

...

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCHv2 0/3] kexec: Add support for UKI format kernel

[Simon Horman]

On Wed, Oct 16, 2024 at 07:34:12PM +0800, Pingfan Liu wrote:
> As a UEFI PE format kernel image becomes more popular, there is a need
> for kexec to reboot those kinds of images.
> 
> After the introduction of the UKI (another PE), at present, there are
> three competitive methods to support that goal, but all of them have
> pros and cons.[1] It seems that none of them can be accepted in the near
> future.  Therefore, we are resorting to the user space kexec-tools to
> parse the UKI format for the time being.
> 
> By parsing the UKI, systemd-stub is stepped around and PCM will not affect
> the boot up of the second system.
> 
> [1]: https://github.com/rhkdump/kexec_uefi/blob/main/overview.md#the-competitive-solutions

Thanks,

Series applied.

Please see my request for a follow-up for patch 2/3 [1].

[1] https://lore.kernel.org/kexec/20241021101646.GA402847@kernel.org/

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCHv2 0/3] kexec: Add support for UKI format kernel

[Philipp Rudo]

Hi Pingfan,
Hi Simon,

I know I'm a little bit too late, but I finally had time to take a
look. All in all the series looks fine to me. But I have two questions.

1) Is there a reason why you don't enable the UKI for x86 as well? The
way I see it patch 1 and 2 cover x86 as well. So you only should need a
patch similar to patch 3. Or am I missing something?

2) Would it make sense to "reuse" the mechanism you introduced for the
initrd for the command line as well. I.e. if it's a UKI with a .cmdline
section and the user didn't provide any of the --command-line,
--reuse-cmdline, etc. then use the command line from the UKI.

Thanks
Philipp

On Wed, 16 Oct 2024 19:34:12 +0800
Pingfan Liu <piliu@redhat.com> wrote:

> As a UEFI PE format kernel image becomes more popular, there is a need
> for kexec to reboot those kinds of images.
> 
> After the introduction of the UKI (another PE), at present, there are
> three competitive methods to support that goal, but all of them have
> pros and cons.[1] It seems that none of them can be accepted in the near
> future.  Therefore, we are resorting to the user space kexec-tools to
> parse the UKI format for the time being.
> 
> By parsing the UKI, systemd-stub is stepped around and PCM will not affect
> the boot up of the second system.
> 
> [1]: https://github.com/rhkdump/kexec_uefi/blob/main/overview.md#the-competitive-solutions
> 
> Cc: Simon Horman <horms@kernel.org>
> Cc: Eric Biederman <ebiederm@xmission.com>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Dave Young <dyoung@redhat.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Jan Hendrik Farr <kernel@jfarr.cc>
> Cc: Philipp Rudo <prudo@redhat.com>
> Cc: Lennart Poettering <mzxreary@0pointer.de>
> Cc: kexec@lists.infradead.org
> ---
> v1 -> v2:
> Add include/pe.h in makefile to fix 'make dist'
> Rename default_initrd_fd to implicit_initrd_fd
> 
> 
> Pingfan Liu (3):
>   kexec: Introduce implicit_initrd_fd to pass internal initrd
>     information
>   kexec: Introduce UKI image parser
>   arm64: Support UKI image format
> 
>  include/Makefile                     |   1 +
>  include/pe.h                         | 104 +++++++++++++++++++++++
>  kexec/Makefile                       |   1 +
>  kexec/arch/arm64/kexec-arm64.c       |   1 +
>  kexec/arch/arm64/kexec-image-arm64.c |   3 +-
>  kexec/arch/x86_64/kexec-bzImage64.c  |   3 +-
>  kexec/kexec-uki.c                    | 122 +++++++++++++++++++++++++++
>  kexec/kexec.c                        |   2 +
>  kexec/kexec.h                        |   5 ++
>  9 files changed, 240 insertions(+), 2 deletions(-)
>  create mode 100644 include/pe.h
>  create mode 100644 kexec/kexec-uki.c
> 


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCHv2 0/3] kexec: Add support for UKI format kernel

[Pingfan Liu]

Hi Philipp,

Thanks for sharing your thoughts.

On Tue, Oct 22, 2024 at 12:25 AM Philipp Rudo <prudo@redhat.com> wrote:
>
> Hi Pingfan,
> Hi Simon,
>
> I know I'm a little bit too late, but I finally had time to take a
> look. All in all the series looks fine to me. But I have two questions.
>
> 1) Is there a reason why you don't enable the UKI for x86 as well? The
> way I see it patch 1 and 2 cover x86 as well. So you only should need a
> patch similar to patch 3. Or am I missing something?
>

Yes, there should be one for x86. I will do it after this series is merged.

> 2) Would it make sense to "reuse" the mechanism you introduced for the
> initrd for the command line as well. I.e. if it's a UKI with a .cmdline
> section and the user didn't provide any of the --command-line,
> --reuse-cmdline, etc. then use the command line from the UKI.
>

I had thought about it, and the scenario should look like this: a
system with a UKI image so there is no direct access to get initrd,
which obstructs the use of --initrd option. On the other hand,
--reuse-cmdline can always get the cmdline from /proc/cmdline without
trouble.

Thanks,

Pingfan

> Thanks
> Philipp
>
> On Wed, 16 Oct 2024 19:34:12 +0800
> Pingfan Liu <piliu@redhat.com> wrote:
>
> > As a UEFI PE format kernel image becomes more popular, there is a need
> > for kexec to reboot those kinds of images.
> >
> > After the introduction of the UKI (another PE), at present, there are
> > three competitive methods to support that goal, but all of them have
> > pros and cons.[1] It seems that none of them can be accepted in the near
> > future.  Therefore, we are resorting to the user space kexec-tools to
> > parse the UKI format for the time being.
> >
> > By parsing the UKI, systemd-stub is stepped around and PCM will not affect
> > the boot up of the second system.
> >
> > [1]: https://github.com/rhkdump/kexec_uefi/blob/main/overview.md#the-competitive-solutions
> >
> > Cc: Simon Horman <horms@kernel.org>
> > Cc: Eric Biederman <ebiederm@xmission.com>
> > Cc: Baoquan He <bhe@redhat.com>
> > Cc: Dave Young <dyoung@redhat.com>
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Cc: Jan Hendrik Farr <kernel@jfarr.cc>
> > Cc: Philipp Rudo <prudo@redhat.com>
> > Cc: Lennart Poettering <mzxreary@0pointer.de>
> > Cc: kexec@lists.infradead.org
> > ---
> > v1 -> v2:
> > Add include/pe.h in makefile to fix 'make dist'
> > Rename default_initrd_fd to implicit_initrd_fd
> >
> >
> > Pingfan Liu (3):
> >   kexec: Introduce implicit_initrd_fd to pass internal initrd
> >     information
> >   kexec: Introduce UKI image parser
> >   arm64: Support UKI image format
> >
> >  include/Makefile                     |   1 +
> >  include/pe.h                         | 104 +++++++++++++++++++++++
> >  kexec/Makefile                       |   1 +
> >  kexec/arch/arm64/kexec-arm64.c       |   1 +
> >  kexec/arch/arm64/kexec-image-arm64.c |   3 +-
> >  kexec/arch/x86_64/kexec-bzImage64.c  |   3 +-
> >  kexec/kexec-uki.c                    | 122 +++++++++++++++++++++++++++
> >  kexec/kexec.c                        |   2 +
> >  kexec/kexec.h                        |   5 ++
> >  9 files changed, 240 insertions(+), 2 deletions(-)
> >  create mode 100644 include/pe.h
> >  create mode 100644 kexec/kexec-uki.c
> >
>


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCHv3 2/3] kexec: Introduce UKI image parser

[Pingfan Liu]

A UKI image is a PE file that consists of several sections, typically
including: .text, .data, .linux, .initrd, .cmdline, and others.

The kernel image is stored in the .linux section, which is one of the
formats currently recognized by kexec-tools. Therefore, the UKI parser
can be used to strip away the UKI layer, allowing the other parser to
continue the procession of the kernel image.

Signed-off-by: Pingfan Liu <piliu@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: kexec@lists.infradead.org
---
 include/Makefile  |   1 +
 include/pe.h      | 104 +++++++++++++++++++++++++++++++++++++++
 kexec/Makefile    |   1 +
 kexec/kexec-uki.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++
 kexec/kexec.h     |   4 ++
 5 files changed, 233 insertions(+)
 create mode 100644 include/pe.h
 create mode 100644 kexec/kexec-uki.c

diff --git a/include/Makefile b/include/Makefile
index cd88a26..6a3e854 100644
--- a/include/Makefile
+++ b/include/Makefile
@@ -8,6 +8,7 @@ dist += include/Makefile		\
 	include/x86/mb_header.h		\
 	include/x86/multiboot2.h	\
 	include/elf.h			\
+	include/pe.h			\
 	include/image.h			\
 	include/unused.h		\
 	include/boot/linuxbios_tables.h	\
diff --git a/include/pe.h b/include/pe.h
new file mode 100644
index 0000000..2617074
--- /dev/null
+++ b/include/pe.h
@@ -0,0 +1,104 @@
+/*
+ * Extract from linux kernel include/linux/pe.h
+ */
+
+#ifndef __PE_H__
+#define __PE_H__
+
+struct pe_hdr {
+	uint32_t magic;		/* PE magic */
+	uint16_t machine;	/* machine type */
+	uint16_t sections;	/* number of sections */
+	uint32_t timestamp;	/* time_t */
+	uint32_t symbol_table;	/* symbol table offset */
+	uint32_t symbols;	/* number of symbols */
+	uint16_t opt_hdr_size;	/* size of optional header */
+	uint16_t flags;		/* flags */
+};
+
+/* the fact that pe32 isn't padded where pe32+ is 64-bit means union won't
+ * work right.  vomit. */
+struct pe32_opt_hdr {
+	/* "standard" header */
+	uint16_t magic;		/* file type */
+	uint8_t  ld_major;	/* linker major version */
+	uint8_t  ld_minor;	/* linker minor version */
+	uint32_t text_size;	/* size of text section(s) */
+	uint32_t data_size;	/* size of data section(s) */
+	uint32_t bss_size;	/* size of bss section(s) */
+	uint32_t entry_point;	/* file offset of entry point */
+	uint32_t code_base;	/* relative code addr in ram */
+	uint32_t data_base;	/* relative data addr in ram */
+	/* "windows" header */
+	uint32_t image_base;	/* preferred load address */
+	uint32_t section_align;	/* alignment in bytes */
+	uint32_t file_align;	/* file alignment in bytes */
+	uint16_t os_major;	/* major OS version */
+	uint16_t os_minor;	/* minor OS version */
+	uint16_t image_major;	/* major image version */
+	uint16_t image_minor;	/* minor image version */
+	uint16_t subsys_major;	/* major subsystem version */
+	uint16_t subsys_minor;	/* minor subsystem version */
+	uint32_t win32_version;	/* reserved, must be 0 */
+	uint32_t image_size;	/* image size */
+	uint32_t header_size;	/* header size rounded up to
+				   file_align */
+	uint32_t csum;		/* checksum */
+	uint16_t subsys;	/* subsystem */
+	uint16_t dll_flags;	/* more flags! */
+	uint32_t stack_size_req;/* amt of stack requested */
+	uint32_t stack_size;	/* amt of stack required */
+	uint32_t heap_size_req;	/* amt of heap requested */
+	uint32_t heap_size;	/* amt of heap required */
+	uint32_t loader_flags;	/* reserved, must be 0 */
+	uint32_t data_dirs;	/* number of data dir entries */
+};
+
+struct pe32plus_opt_hdr {
+	uint16_t magic;		/* file type */
+	uint8_t  ld_major;	/* linker major version */
+	uint8_t  ld_minor;	/* linker minor version */
+	uint32_t text_size;	/* size of text section(s) */
+	uint32_t data_size;	/* size of data section(s) */
+	uint32_t bss_size;	/* size of bss section(s) */
+	uint32_t entry_point;	/* file offset of entry point */
+	uint32_t code_base;	/* relative code addr in ram */
+	/* "windows" header */
+	uint64_t image_base;	/* preferred load address */
+	uint32_t section_align;	/* alignment in bytes */
+	uint32_t file_align;	/* file alignment in bytes */
+	uint16_t os_major;	/* major OS version */
+	uint16_t os_minor;	/* minor OS version */
+	uint16_t image_major;	/* major image version */
+	uint16_t image_minor;	/* minor image version */
+	uint16_t subsys_major;	/* major subsystem version */
+	uint16_t subsys_minor;	/* minor subsystem version */
+	uint32_t win32_version;	/* reserved, must be 0 */
+	uint32_t image_size;	/* image size */
+	uint32_t header_size;	/* header size rounded up to
+				   file_align */
+	uint32_t csum;		/* checksum */
+	uint16_t subsys;	/* subsystem */
+	uint16_t dll_flags;	/* more flags! */
+	uint64_t stack_size_req;/* amt of stack requested */
+	uint64_t stack_size;	/* amt of stack required */
+	uint64_t heap_size_req;	/* amt of heap requested */
+	uint64_t heap_size;	/* amt of heap required */
+	uint32_t loader_flags;	/* reserved, must be 0 */
+	uint32_t data_dirs;	/* number of data dir entries */
+};
+
+struct section_header {
+	char name[8];			/* name or "/12\0" string tbl offset */
+	uint32_t virtual_size;		/* size of loaded section in ram */
+	uint32_t virtual_address;	/* relative virtual address */
+	uint32_t raw_data_size;		/* size of the section */
+	uint32_t data_addr;		/* file pointer to first page of sec */
+	uint32_t relocs;		/* file pointer to relocation entries */
+	uint32_t line_numbers;		/* line numbers! */
+	uint16_t num_relocs;		/* number of relocations */
+	uint16_t num_lin_numbers;	/* srsly. */
+	uint32_t flags;
+};
+
+#endif
diff --git a/kexec/Makefile b/kexec/Makefile
index 11682bf..d4f26d7 100644
--- a/kexec/Makefile
+++ b/kexec/Makefile
@@ -18,6 +18,7 @@ KEXEC_SRCS_base += kexec/kexec-elf-core.c
 KEXEC_SRCS_base += kexec/kexec-elf-rel.c
 KEXEC_SRCS_base += kexec/kexec-elf-boot.c
 KEXEC_SRCS_base += kexec/kexec-pe-zboot.c
+KEXEC_SRCS_base += kexec/kexec-uki.c
 KEXEC_SRCS_base += kexec/kexec-iomem.c
 KEXEC_SRCS_base += kexec/firmware_memmap.c
 KEXEC_SRCS_base += kexec/crashdump.c
diff --git a/kexec/kexec-uki.c b/kexec/kexec-uki.c
new file mode 100644
index 0000000..210ebb6
--- /dev/null
+++ b/kexec/kexec-uki.c
@@ -0,0 +1,123 @@
+#include <limits.h>
+#include <stdint.h>
+#include <string.h>
+#include <stdio.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <pe.h>
+#include "kexec.h"
+
+#define UKI_LINUX_SECTION ".linux"
+#define UKI_INITRD_SECTION ".initrd"
+#define UKI_CMDLINE_SECTION ".cmdline"
+#define UKI_DTB_SECTION ".dtb"
+
+#define FILENAME_UKI_INITRD          "/tmp/InitrdXXXXXX"
+
+static int embeded_linux_format_index = -1;
+
+/*
+ * Return -1 if not PE, else offset of the PE header
+ */
+static int get_pehdr_offset(const char *buf)
+{
+	int pe_hdr_offset;
+
+	pe_hdr_offset = *((int *)(buf + 0x3c));
+	buf += pe_hdr_offset;
+	if (!!memcmp(buf, "PE\0\0", 4)) {
+		printf("Not a PE file\n");
+		return -1;
+	}
+
+	return pe_hdr_offset;
+}
+
+int uki_image_probe(const char *file_buf, off_t buf_sz)
+{
+	struct pe_hdr *pe_hdr;
+	struct pe32plus_opt_hdr *opt_hdr;
+	struct section_header *sect_hdr;
+	int pe_hdr_offset, section_nr, linux_sz = -1;
+	char *pe_part_buf, *linux_src;
+	char *initrd_fname = NULL;
+	int initrd_fd = -1;
+
+	pe_hdr_offset = get_pehdr_offset(file_buf);
+	pe_part_buf = (char *)file_buf + pe_hdr_offset;
+	pe_hdr = (struct pe_hdr *)pe_part_buf;
+	if (pe_hdr->opt_hdr_size == 0) {
+		printf("ERR: optional header is missing\n");
+		return -1;
+	}
+	section_nr = pe_hdr->sections;
+	opt_hdr = (struct pe32plus_opt_hdr *)(pe_part_buf + sizeof(struct pe_hdr));
+	sect_hdr = (struct section_header *)((char *)opt_hdr + pe_hdr->opt_hdr_size);
+
+	for (int i = 0; i < section_nr; i++) {
+		if (!strcmp(sect_hdr->name, UKI_LINUX_SECTION)) {
+			/* data_addr is relative to the whole file */
+			linux_src = (char *)file_buf + sect_hdr->data_addr;
+			linux_sz = sect_hdr->raw_data_size;
+
+		} else if (!strcmp(sect_hdr->name, UKI_INITRD_SECTION)) {
+			if (!(initrd_fname = strdup(FILENAME_UKI_INITRD))) {
+				dbgprintf("%s: Can't duplicate strings\n", __func__);
+				goto next;
+			}
+
+			if ((initrd_fd = mkstemp(initrd_fname)) < 0) {
+				dbgprintf("%s: Can't open file %s\n", __func__,	initrd_fname);
+				goto next;
+			}
+
+			if (write(initrd_fd, (char *)file_buf + sect_hdr->data_addr,
+					sect_hdr->raw_data_size) != sect_hdr->raw_data_size) {
+				dbgprintf("%s: Can't write the compressed file %s\n",
+						__func__, initrd_fname);
+				close(initrd_fd);
+				goto next;
+			} else {
+				implicit_initrd_fd = open(initrd_fname, O_RDONLY);
+				close(initrd_fd);
+			}
+		}
+next:
+		sect_hdr++;
+	}
+
+	if (linux_sz == -1) {
+		printf("ERR: can not find .linux section\n");
+		return -1;
+	}
+	/*
+	 * After stripping the UKI coat, the real kernel format can be handled now.
+	 */
+	for (int i = 0; i < file_types; i++) {
+		/* kernel_fd will be created by probe */
+		if (file_type[i].probe != uki_image_probe &&
+		    file_type[i].probe(linux_src, linux_sz) >= 0) {
+			embeded_linux_format_index = i;
+			break;
+		}
+	}
+	if (embeded_linux_format_index < 0) {
+		printf("Can not recognize the kernel format in .linux section\n");
+		return -1;
+	}
+	return 0;
+}
+
+int uki_image_load(int argc, char **argv, const char *buf, off_t len,
+			struct kexec_info *info)
+{
+	return file_type[embeded_linux_format_index].load(argc, argv, buf, len, info);
+}
+
+void uki_image_usage(void)
+{
+	printf(
+"     An UKI image.\n");
+}
diff --git a/kexec/kexec.h b/kexec/kexec.h
index e70c18d..a2e19c4 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -357,4 +357,8 @@ static inline void ultoa(unsigned long val, char *str)
 	str[pos] = 0;
 }
 
+extern int uki_image_probe(const char *file_buf, off_t buf_sz);
+extern int uki_image_load(int argc, char **argv, const char *buf, off_t len,
+		struct kexec_info *info);
+extern void uki_image_usage(void);
 #endif /* KEXEC_H */
-- 
2.41.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCHv2 2/3] kexec: Introduce UKI image parser

[Pingfan Liu]

Hi Simon,

Thanks for your review.

On Mon, Oct 21, 2024 at 6:16 PM Simon Horman <horms@kernel.org> wrote:
>
> On Wed, Oct 16, 2024 at 07:34:14PM +0800, Pingfan Liu wrote:
> > A UKI image is a PE file that consists of several sections, typically
> > including: .text, .data, .linux, .initrd, .cmdline, and others.
> >
> > The kernel image is stored in the .linux section, which is one of the
> > formats currently recognized by kexec-tools. Therefore, the UKI parser
> > can be used to strip away the UKI layer, allowing the other parser to
> > continue the procession of the kernel image.
> >
> > Signed-off-by: Pingfan Liu <piliu@redhat.com>
>
> ...
>
> > diff --git a/kexec/kexec-uki.c b/kexec/kexec-uki.c
>
> ...
>
> > +int uki_image_probe(const char *file_buf, off_t buf_sz)
> > +{
> > +     struct pe_hdr *pe_hdr;
> > +     struct pe32plus_opt_hdr *opt_hdr;
> > +     struct section_header *sect_hdr;
> > +     int pe_hdr_offset, section_nr, linux_sz = -1;
> > +     char *pe_part_buf, *linux_src;
> > +     char *initrd_fname = NULL;
> > +     int initrd_fd = -1;
> > +
> > +     pe_hdr_offset = get_pehdr_offset(file_buf);
> > +     pe_part_buf = (char *)file_buf + pe_hdr_offset;
> > +     pe_hdr = (struct pe_hdr *)pe_part_buf;
> > +     if (pe_hdr->opt_hdr_size == 0) {
> > +             printf("ERR: optional header is missing\n");
> > +             return -1;
> > +     }
> > +     section_nr = pe_hdr->sections;
> > +     opt_hdr = (struct pe32plus_opt_hdr *)(pe_part_buf + sizeof(struct pe_hdr));
> > +     sect_hdr = (struct section_header *)((char *)opt_hdr + pe_hdr->opt_hdr_size);
> > +
> > +     for (int i = 0; i < section_nr; i++) {
> > +             if (!strcmp(sect_hdr->name, UKI_LINUX_SECTION)) {
> > +                     /* data_addr is relative to the whole file */
> > +                     linux_src = (char *)file_buf + sect_hdr->data_addr;
> > +                     linux_sz = sect_hdr->raw_data_size;
> > +
> > +             } else if (!strcmp(sect_hdr->name, UKI_INITRD_SECTION)) {
> > +                     if (!(initrd_fname = strdup(FILENAME_UKI_INITRD))) {
> > +                             dbgprintf("%s: Can't duplicate strings\n", __func__);
> > +                             goto next;
> > +                     }
> > +
> > +                     if ((initrd_fd = mkstemp(initrd_fname)) < 0) {
> > +                             dbgprintf("%s: Can't open file %s\n", __func__, initrd_fname);
> > +                             goto next;
> > +                     }
> > +
> > +                     if (write(initrd_fd, (char *)file_buf + sect_hdr->data_addr,
> > +                                     sect_hdr->raw_data_size) != sect_hdr->raw_data_size) {
> > +                             dbgprintf("%s: Can't write the compressed file %s\n",
> > +                                             __func__, initrd_fname);
> > +                             goto next;
>
> I plan to apply this series, but initrd_fd appears to be leaked here.
> Could you send a follow-up patch to address that? Perhaps like this:
>
I have sent out a dedicated v3 "[PATCHv3 2/3] kexec: Introduce UKI
image parser" to fix this leak issue.

>                         err = write(...);
>                         close(initrd_fd)

My fix duplicates the close(initrd_fd) in each branch. Since initrd_fd
is the only reference to mkstemp(initrd_fname), I think if closing it,
the temp file may disappear before implicit_initrd_fd=
open(initrd_fname, O_RDONLY).


Best Regards,

Pingfan

>                         if (err != sect_hdr->raw_data_size) {
>                                 ...
>                         } else {
>                                 ...
>                         }
>
> Thanks!
>
> > +                     } else {
> > +                             implicit_initrd_fd = open(initrd_fname, O_RDONLY);
> > +                             close(initrd_fd);
> > +                     }
> > +             }
> > +next:
> > +             sect_hdr++;
> > +     }
>
> ...
>


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec