[PATCH v2 0/5] kexec-tools: multiboot2: Accept x86-64 images

[PATCH v2 0/5] kexec-tools: multiboot2: Accept x86-64 images

[Zhaofeng Li]

Hi Simon,

Thanks for your review! In this patchset I have addressed your comments
and included an extra fix.

Zhaofeng

--

This patch series adds support for loading x86-64 ELF images with
the multiboot2 header and fixes a couple of related issues.

Changes since v1:
- Coding style fixes
- Fixed one more occurrence of mhi.rel_tag in rel_min/rel_max patch
- Added a patch to address code duplication in elf_x86_64_probe
- Added a patch to address the case where add_buffer tried to
  allocate at 0x0.

Zhaofeng Li (5):
  x86: Consolidate elf_x86_probe routines
  multiboot2: Correct MBI size calculation
  multiboot2: Use rel_min and rel_max for buffer destinations
  multiboot2: Avoid first 0x500 bytes
  multiboot2: Accept x86-64 images

 kexec/arch/i386/kexec-elf-x86.c      | 44 +++++++++++++++++++++++-----
 kexec/arch/i386/kexec-mb2-x86.c      | 29 ++++++++++++------
 kexec/arch/i386/kexec-x86.h          |  1 +
 kexec/arch/x86_64/kexec-elf-x86_64.c | 28 ++----------------
 kexec/arch/x86_64/kexec-x86_64.c     |  4 +--
 5 files changed, 62 insertions(+), 44 deletions(-)

-- 
2.32.0


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

[PATCH v2 1/5] x86: Consolidate elf_x86_probe routines

[Zhaofeng Li]

Signed-off-by: Zhaofeng Li <hello@zhaofeng.li>
---
 kexec/arch/i386/kexec-elf-x86.c      | 44 +++++++++++++++++++++++-----
 kexec/arch/i386/kexec-x86.h          |  1 +
 kexec/arch/x86_64/kexec-elf-x86_64.c | 28 ++----------------
 3 files changed, 40 insertions(+), 33 deletions(-)

diff --git a/kexec/arch/i386/kexec-elf-x86.c b/kexec/arch/i386/kexec-elf-x86.c
index fedf031..8eba242 100644
--- a/kexec/arch/i386/kexec-elf-x86.c
+++ b/kexec/arch/i386/kexec-elf-x86.c
@@ -42,7 +42,7 @@
 
 static const int probe_debug = 0;
 
-int elf_x86_probe(const char *buf, off_t len)
+int elf_x86_any_probe(const char *buf, off_t len, enum coretype arch)
 {
 	
 	struct mem_ehdr ehdr;
@@ -56,20 +56,50 @@ int elf_x86_probe(const char *buf, off_t len)
 	}
 
 	/* Verify the architecuture specific bits */
-	if ((ehdr.e_machine != EM_386) && (ehdr.e_machine != EM_486)) {
-		/* for a different architecture */
-		if (probe_debug) {
-			fprintf(stderr, "Not i386 ELF executable\n");
+	switch (arch) {
+	case CORE_TYPE_ELF32:
+		if ((ehdr.e_machine != EM_386) && (ehdr.e_machine != EM_486)) {
+			if (probe_debug)
+				fprintf(stderr, "Not i386 ELF executable\n");
+			result = -1;
+			goto out;
 		}
-		result = -1;
-		goto out;
+		break;
+
+	case CORE_TYPE_ELF64:
+		if (ehdr.e_machine != EM_X86_64) {
+			if (probe_debug)
+				fprintf(stderr, "Not x86_64 ELF executable\n");
+			result = -1;
+			goto out;
+		}
+		break;
+
+	case CORE_TYPE_UNDEF:
+	default:
+		if (
+			(ehdr.e_machine != EM_386) &&
+			(ehdr.e_machine != EM_486) &&
+			(ehdr.e_machine != EM_X86_64)
+		) {
+			if (probe_debug)
+				fprintf(stderr, "Not i386 or x86_64 ELF executable\n");
+			result = -1;
+			goto out;
+		}
+		break;
 	}
+
 	result = 0;
  out:
 	free_elf_info(&ehdr);
 	return result;
 }
 
+int elf_x86_probe(const char *buf, off_t len) {
+	return elf_x86_any_probe(buf, len, CORE_TYPE_ELF32);
+}
+
 void elf_x86_usage(void)
 {
 	printf(	"    --command-line=STRING Set the kernel command line to STRING\n"
diff --git a/kexec/arch/i386/kexec-x86.h b/kexec/arch/i386/kexec-x86.h
index 0f941df..71e4329 100644
--- a/kexec/arch/i386/kexec-x86.h
+++ b/kexec/arch/i386/kexec-x86.h
@@ -66,6 +66,7 @@ void multiboot2_x86_usage(void);
 int multiboot2_x86_probe(const char *buf, off_t buf_len);
 
 int elf_x86_probe(const char *buf, off_t len);
+int elf_x86_any_probe(const char *buf, off_t len, enum coretype arch);
 int elf_x86_load(int argc, char **argv, const char *buf, off_t len,
 	struct kexec_info *info);
 void elf_x86_usage(void);
diff --git a/kexec/arch/x86_64/kexec-elf-x86_64.c b/kexec/arch/x86_64/kexec-elf-x86_64.c
index ad22311..7f9540a 100644
--- a/kexec/arch/x86_64/kexec-elf-x86_64.c
+++ b/kexec/arch/x86_64/kexec-elf-x86_64.c
@@ -37,37 +37,13 @@
 #include "../../kexec-elf-boot.h"
 #include "../i386/x86-linux-setup.h"
 #include "kexec-x86_64.h"
+#include "../i386/kexec-x86.h"
 #include "../i386/crashdump-x86.h"
 #include <arch/options.h>
 
-static const int probe_debug = 0;
-
 int elf_x86_64_probe(const char *buf, off_t len)
 {
-	
-	struct mem_ehdr ehdr;
-	int result;
-	result = build_elf_exec_info(buf, len, &ehdr, 0);
-	if (result < 0) {
-		if (probe_debug) {
-			fprintf(stderr, "Not an ELF executable\n");
-		}
-		goto out;
-	}
-
-	/* Verify the architecuture specific bits */
-	if (ehdr.e_machine != EM_X86_64) {
-		/* for a different architecture */
-		if (probe_debug) {
-			fprintf(stderr, "Not x86_64 ELF executable\n");
-		}
-		result = -1;
-		goto out;
-	}
-	result = 0;
- out:
-	free_elf_info(&ehdr);
-	return result;
+	return elf_x86_any_probe(buf, len, CORE_TYPE_ELF64);
 }
 
 void elf_x86_64_usage(void)
-- 
2.32.0


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

[PATCH v2 2/5] multiboot2: Correct MBI size calculation

[Zhaofeng Li]

tag_load_base_addr is dependent on rel_tag, and tag_framebuffer was
not accounted for.

Signed-off-by: Zhaofeng Li <hello@zhaofeng.li>
---
 kexec/arch/i386/kexec-mb2-x86.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/kexec/arch/i386/kexec-mb2-x86.c b/kexec/arch/i386/kexec-mb2-x86.c
index b4996bc..6bbe35e 100644
--- a/kexec/arch/i386/kexec-mb2-x86.c
+++ b/kexec/arch/i386/kexec-mb2-x86.c
@@ -115,17 +115,26 @@ void multiboot2_x86_usage(void)
 static size_t
 multiboot2_get_mbi_size(int ranges, int cmdline_size, int modcount, int modcmd_size)
 {
-	return (2 * sizeof (uint32_t) + sizeof (struct multiboot_tag)
-		+ sizeof (struct multiboot_tag)
+	size_t mbi_size;
+
+	mbi_size = (2 * sizeof (uint32_t) /* u32 total_size, u32 reserved */
 		+ ALIGN_UP (sizeof (struct multiboot_tag_basic_meminfo), MULTIBOOT_TAG_ALIGN)
 		+ ALIGN_UP ((sizeof (struct multiboot_tag_mmap)
 			+ ranges * sizeof (struct multiboot_mmap_entry)), MULTIBOOT_TAG_ALIGN)
-		+ ALIGN_UP (sizeof (struct multiboot_tag_load_base_addr), MULTIBOOT_TAG_ALIGN)
 		+ (sizeof (struct multiboot_tag_string)
 			+ ALIGN_UP (cmdline_size, MULTIBOOT_TAG_ALIGN))
 		+ (sizeof (struct multiboot_tag_string)
 			+ ALIGN_UP (strlen(BOOTLOADER " " BOOTLOADER_VERSION) + 1, MULTIBOOT_TAG_ALIGN))
-		+ (modcount * sizeof (struct multiboot_tag_module) + modcmd_size));
+		+ (modcount * sizeof (struct multiboot_tag_module) + modcmd_size))
+		+ sizeof (struct multiboot_tag); /* end tag */
+
+	if (mhi.rel_tag)
+		mbi_size += ALIGN_UP (sizeof (struct multiboot_tag_load_base_addr), MULTIBOOT_TAG_ALIGN);
+
+	if (mhi.fb_tag)
+		mbi_size += ALIGN_UP (sizeof (struct multiboot_tag_framebuffer), MULTIBOOT_TAG_ALIGN);
+
+	return mbi_size;
 }
 
 static void multiboot2_read_header_tags(void)
-- 
2.32.0


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

[PATCH v2 3/5] multiboot2: Use rel_min and rel_max for buffer destinations

[Zhaofeng Li]

This would segfault if mhi.rel_tag didn't exist.

Signed-off-by: Zhaofeng Li <hello@zhaofeng.li>
---
 kexec/arch/i386/kexec-mb2-x86.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kexec/arch/i386/kexec-mb2-x86.c b/kexec/arch/i386/kexec-mb2-x86.c
index 6bbe35e..f3c6438 100644
--- a/kexec/arch/i386/kexec-mb2-x86.c
+++ b/kexec/arch/i386/kexec-mb2-x86.c
@@ -582,7 +582,7 @@ int multiboot2_x86_load(int argc, char **argv, const char *buf, off_t len,
 
 			/* Pick the next aligned spot to load it in. Always page align. */
 			addr = add_buffer(info, buf, mod_size, mod_size, getpagesize(),
-					  mhi.rel_tag->min_addr, mhi.rel_tag->max_addr, 1);
+					  rel_min, rel_max, 1);
 
 			/* Add the module command line */
 			sprintf(mod_clp, "%s", mod_command_line);
@@ -602,7 +602,7 @@ int multiboot2_x86_load(int argc, char **argv, const char *buf, off_t len,
 		return -1;
 
 	addr = add_buffer(info, mbi_buf, mbi_bytes, mbi_bytes, 4,
-			  mhi.rel_tag->min_addr, mhi.rel_tag->max_addr, 1);
+			  rel_min, rel_max, 1);
 
 	elf_rel_get_symbol(&info->rhdr, "entry32_regs", &regs, sizeof(regs));
 	regs.eax = MULTIBOOT2_BOOTLOADER_MAGIC;
-- 
2.32.0


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

[PATCH v2 4/5] multiboot2: Avoid first 0x500 bytes

[Zhaofeng Li]

In some cases, add_buffer will actually try to allocate the buffer
at 0x0, which may not be acceptable by some kernels. Let's avoid
the first 0x500 bytes so we don't screw up the IVT and BDA.

Signed-off-by: Zhaofeng Li <hello@zhaofeng.li>
---
 kexec/arch/i386/kexec-mb2-x86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kexec/arch/i386/kexec-mb2-x86.c b/kexec/arch/i386/kexec-mb2-x86.c
index f3c6438..f3e6b38 100644
--- a/kexec/arch/i386/kexec-mb2-x86.c
+++ b/kexec/arch/i386/kexec-mb2-x86.c
@@ -526,7 +526,7 @@ int multiboot2_x86_load(int argc, char **argv, const char *buf, off_t len,
 		rel_min = lu_start + 4096;
 		rel_max = lu_end;
 	} else {
-		rel_min = 0;
+		rel_min = 0x500;
 		rel_max = ULONG_MAX;
 	}
 
-- 
2.32.0


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

[PATCH v2 5/5] multiboot2: Accept x86-64 images

[Zhaofeng Li]

Signed-off-by: Zhaofeng Li <hello@zhaofeng.li>
---
 kexec/arch/i386/kexec-mb2-x86.c  | 6 ++++--
 kexec/arch/x86_64/kexec-x86_64.c | 4 ++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/kexec/arch/i386/kexec-mb2-x86.c b/kexec/arch/i386/kexec-mb2-x86.c
index f3e6b38..36fef20 100644
--- a/kexec/arch/i386/kexec-mb2-x86.c
+++ b/kexec/arch/i386/kexec-mb2-x86.c
@@ -76,8 +76,10 @@ int multiboot2_x86_probe(const char *buf, off_t buf_len)
 /* Is it a good idea to try booting this file? */
 {
 	int i, len;
-	/* First of all, check that this is an ELF file */
-	if ((i=elf_x86_probe(buf, buf_len)) < 0)
+
+	/* First of all, check that this is an ELF file for either x86 or x86-64 */
+	i = elf_x86_any_probe(buf, buf_len, CORE_TYPE_UNDEF);
+	if (i < 0)
 		return i;
 
 	/* Now look for a multiboot header. */
diff --git a/kexec/arch/x86_64/kexec-x86_64.c b/kexec/arch/x86_64/kexec-x86_64.c
index 394cfca..ffd84f0 100644
--- a/kexec/arch/x86_64/kexec-x86_64.c
+++ b/kexec/arch/x86_64/kexec-x86_64.c
@@ -33,11 +33,11 @@
 #include <arch/options.h>
 
 struct file_type file_type[] = {
+	{ "multiboot2-x86", multiboot2_x86_probe, multiboot2_x86_load,
+	  multiboot2_x86_usage },
 	{ "elf-x86_64", elf_x86_64_probe, elf_x86_64_load, elf_x86_64_usage },
 	{ "multiboot-x86", multiboot_x86_probe, multiboot_x86_load,
 	  multiboot_x86_usage },
-	{ "multiboot2-x86", multiboot2_x86_probe, multiboot2_x86_load,
-	  multiboot2_x86_usage },
 	{ "elf-x86", elf_x86_probe, elf_x86_load, elf_x86_usage },
 	{ "bzImage64", bzImage64_probe, bzImage64_load, bzImage64_usage },
 	{ "bzImage", bzImage_probe, bzImage_load, bzImage_usage },
-- 
2.32.0


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

Re: [PATCH v2 0/5] kexec-tools: multiboot2: Accept x86-64 images

[Simon Horman]

On Mon, Sep 13, 2021 at 08:51:37PM -0700, Zhaofeng Li wrote:
> Hi Simon,
> 
> Thanks for your review! In this patchset I have addressed your comments
> and included an extra fix.

Thanks, much appreciated.

I have applied the series.

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