[PATCH] xen/x86: fix xen.efi boot crash from some bootloaders

["Yann Sionneau" <yann.sionneau@xxxxxxxxxx>]

xen.efi PE does not boot when loaded from shim or some patched
downstream grub2.

What happens is the bootloader would honour the MEM_DISCARDABLE
flag of the .reloc section meaning it would not load its content
into memory.

But Xen is parsing the .reloc section content twice at boot:
* https://elixir.bootlin.com/xen/v4.20.1/source/xen/common/efi/boot.c#L1362
* https://elixir.bootlin.com/xen/v4.20.1/source/xen/arch/x86/efi/efi-boot.h#L237

Therefore it would crash with the following message:
"Unsupported relocation type" as reported there:

* https://github.com/QubesOS/qubes-issues/issues/8206#issuecomment-2619048838
* 
https://lore.kernel.org/xen-devel/7e039262-1f54-46e1-8f70-ac3f03607d5a@xxxxxxxx/T/#me122b9e6c27cd98db917da2c9f67e74a2c6ad7a5

This commit adds a small C host tool named keeprelocs
that is called after xen.efi is produced by the build system
in order to remove this bit from its .reloc section header.

Signed-off-by: Yann Sionneau <yann.sionneau@xxxxxxxxxx>
---
 xen/Makefile           |   5 +-
 xen/arch/x86/Makefile  |   1 +
 xen/tools/Makefile     |   3 ++
 xen/tools/keeprelocs.c | 119 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 127 insertions(+), 1 deletion(-)
 create mode 100644 xen/tools/keeprelocs.c

diff --git a/xen/Makefile b/xen/Makefile
index 8fc4e042ff..7dc9cd7e05 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -299,10 +299,13 @@ export XEN_HAS_CHECKPOLICY := $(call 
success,$(CHECKPOLICY) -h 2>&1 | grep -q xe
 # ===========================================================================
 # Rules shared between *config targets and build targets
 
-PHONY += tools_fixdep
+PHONY += tools_fixdep tools_keeprelocs
 tools_fixdep:
        $(Q)$(MAKE) $(build)=tools tools/fixdep
 
+tools_keeprelocs:
+       $(Q)$(MAKE) $(build)=tools tools/keeprelocs
+
 PHONY += outputmakefile
 # Before starting out-of-tree build, make sure the source tree is clean.
 # outputmakefile generates a Makefile in the output directory, if using a
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index ce724a9daa..9a47002fae 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -236,6 +236,7 @@ endif
        $(NM) -pa --format=sysv $@ \
                | $(objtree)/tools/symbols --all-symbols --xensyms --sysv 
--sort \
                > $@.map
+       $(objtree)/tools/keeprelocs -q -i $@
 ifeq ($(CONFIG_DEBUG_INFO),y)
        $(if $(filter --strip-debug,$(EFI_LDFLAGS)),:$(space))$(OBJCOPY) -O 
elf64-x86-64 $@ $@.elf
 endif
diff --git a/xen/tools/Makefile b/xen/tools/Makefile
index a5078b7cb8..4fd917b398 100644
--- a/xen/tools/Makefile
+++ b/xen/tools/Makefile
@@ -1,2 +1,5 @@
 hostprogs-always-y += symbols
 hostprogs-always-y += fixdep
+hostprogs-always-$(XEN_BUILD_PE) += keeprelocs
+# next line is to allow including include/efi/pe.h
+HOSTCFLAGS_keeprelocs.o := -I ../include
\ No newline at end of file
diff --git a/xen/tools/keeprelocs.c b/xen/tools/keeprelocs.c
new file mode 100644
index 0000000000..c169ddba1a
--- /dev/null
+++ b/xen/tools/keeprelocs.c
@@ -0,0 +1,119 @@
+#include <stdio.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <efi/pe.h>
+
+#undef DEBUG
+
+static void print_usage(const char *name) {
+       printf("%s: [-q] [-h] -i xen.efi\n", name);
+}
+
+int main(int argc, char **argv)
+{
+       char *filename = NULL;
+       int fd;
+       char *mem;
+       struct stat st;
+       off_t len;
+       int ret;
+       struct mz_hdr *mz;
+       struct pe_hdr *pe;
+       int opt;
+       const char *prog_name = argv[0];
+       int quiet = 0;
+
+       while ((opt = getopt(argc, argv, ":i:qh")) != -1)
+       {
+               switch (opt) {
+               case 'i':
+                       filename = optarg;
+                       break;
+               case 'q':
+                       quiet = 1;
+                       break;
+               case 'h':
+                       print_usage(prog_name);
+                       return 0;
+                       break;
+               case '?':
+               default:
+                       print_usage(prog_name);
+                       return -1;
+               }
+       }
+
+
+       if (!filename) {
+               printf("Error: you must provide a `-i xen.efi` argument\n");
+               return -1;
+       }
+
+       fd = open(filename, O_RDWR);
+       if (fd < 0) {
+               printf("Could not open file %s: %s\n", filename, 
strerror(errno));
+               return -1;
+       }
+
+       ret = fstat(fd, &st);
+       if (ret < 0) {
+               perror("Error while getting PE file length");
+               return -1;
+       }
+
+       len = st.st_size;
+       mem = mmap(NULL, len, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+
+       if (mem == MAP_FAILED) {
+               perror("Failed to mmap PE file");
+               return -1;
+       }
+
+       mz = (struct mz_hdr *)mem;
+       if (mz->magic != MZ_MAGIC) { // "MZ"
+               printf("file has incorrect MZ header 0x%02x instead of 
0x5a4d\n", mz->magic);
+               return -1;
+       }
+
+       pe = (struct pe_hdr *)(mem + mz->peaddr);
+       if (strncmp((char *)&pe->magic, "PE\0\0", 4)) {
+               printf("file has incorrect PE header magic %08x instead of 
0x00004550\n", pe->magic);
+               return -1;
+       }
+
+       if (pe->opt_hdr_size == 0) {
+               printf("file has empty OptionalHeader\n");
+               return -1;
+       }
+
+       struct section_header *section = (struct section_header *)((uint8_t 
*)pe + sizeof(*pe) + pe->opt_hdr_size);
+       for (unsigned int section_id = 0; section_id < pe->sections; 
section_id++, section++)
+       {
+#ifdef DEBUG
+               printf("section %s\n", section->Name);
+#endif
+               if (strncmp(section->name, ".reloc", strlen(".reloc")))
+                       continue;
+
+               if (!quiet)
+                       printf(".reloc section characteristics: %08x\n", 
section->flags);
+               if (section->flags & IMAGE_SCN_MEM_DISCARDABLE) {
+                       if (!quiet)
+                               printf("MEM_DISCARDABLE flag found! Dropping 
it.\n");
+                       section->flags &= ~(IMAGE_SCN_MEM_DISCARDABLE);
+               }
+       }
+
+       munmap(mem, len);
+       close(fd);
+
+       if (!quiet)
+               printf("Ok!\n");
+       return 0;
+}
-- 
2.43.0



Yann Sionneau | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech