Fix aa64 page fault with EFI_MEMORY_ATTRIBUTE_PROTOCOL

Signed-off-by: Robbie Harwood <rharwood@redhat.com>

0329-grub_dl_set_mem_attrs-fix-format-string.patch

@@ -0,0 +1,38 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Laszlo Ersek <lersek@redhat.com>
+Date: Fri, 7 Apr 2023 14:54:35 +0200
+Subject: [PATCH] grub_dl_set_mem_attrs(): fix format string
+
+The grub_dprintf() call for printing the message
+
+  updating attributes for GOT and trampolines
+
+passes the argument "mod->name", but the format string doesn't accept that
+argument.
+
+Print the module name too.
+
+Example output:
+
+> kern/dl.c:736: updating attributes for GOT and trampolines ("video_fb")
+
+Fixes: ad1b904d325b (nx: set page permissions for loaded modules.)
+Signed-off-by: Laszlo Ersek <lersek@redhat.com>
+---
+ grub-core/kern/dl.c | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c
+index ab9101a5ad..a97f4a8b13 100644
+--- a/grub-core/kern/dl.c
++++ b/grub-core/kern/dl.c
+@@ -733,7 +733,8 @@ grub_dl_set_mem_attrs (grub_dl_t mod, void *ehdr)
+     {
+       tgsz = ALIGN_UP(tgsz, arch_addralign);
+ 
+-      grub_dprintf ("modules", "updating attributes for GOT and trampolines\n",
++      grub_dprintf ("modules",
++		    "updating attributes for GOT and trampolines (\"%s\")\n",
+ 		    mod->name);
+       grub_update_mem_attrs (tgaddr, tgsz, GRUB_MEM_ATTR_R|GRUB_MEM_ATTR_X,
+ 			     GRUB_MEM_ATTR_W);

0330-grub_dl_set_mem_attrs-add-self-check-for-the-tramp-G.patch

@@ -0,0 +1,140 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Laszlo Ersek <lersek@redhat.com>
+Date: Fri, 7 Apr 2023 16:21:54 +0200
+Subject: [PATCH] grub_dl_set_mem_attrs(): add self-check for the tramp/GOT
+ sizes
+
+On aarch64 UEFI, we currently have a crasher:
+
+  grub_dl_load_core()
+    grub_dl_load_core_noinit()
+
+      /* independent allocation: must remain writable */
+      mod = grub_zalloc();
+
+      /* allocates module image with incorrect tail alignment */
+      grub_dl_load_segments()
+
+      /* write-protecting the module image makes "mod" read-only! */
+      grub_dl_set_mem_attrs()
+        grub_update_mem_attrs()
+
+    grub_dl_init()
+      /* page fault, crash */
+      mod->next = ...;
+
+- Commit 887f1d8fa976 ("modules: load module sections at page-aligned
+  addresses", 2023-02-08) forgot to page-align the allocation of the
+  trampolines and GOT areas of grub2 modules, in grub_dl_load_segments().
+
+- Commit ad1b904d325b ("nx: set page permissions for loaded modules.",
+  2023-02-08) calculated a common bounding box for the trampolines and GOT
+  areas in grub_dl_set_mem_attrs(), rounded the box size up to a whole
+  multiple of EFI page size ("arch_addralign"), and write-protected the
+  resultant page range.
+
+Consequently, grub_dl_load_segments() places the module image in memory
+such that its tail -- the end of the trampolines and GOT areas -- lands at
+the head of a page whose tail in turn contains independent memory
+allocations, such as "mod". grub_dl_set_mem_attrs() will then unwittingly
+write-protect these other allocations too.
+
+But "mod" must remain writable: we assign "mod->next" in grub_dl_init()
+subsequently. Currently we crash there with a page fault / permission
+fault.
+
+(The crash is not trivial to hit: the tramp/GOT areas are irrelevant on
+x86_64, plus the page protection depends on the UEFI platform firmware
+providing EFI_MEMORY_ATTRIBUTE_PROTOCOL. In practice, the crash is
+restricted to aarch64 edk2 (ArmVirtQemu) builds containing commit
+1c4dfadb4611, "ArmPkg/CpuDxe: Implement EFI memory attributes protocol",
+2023-03-16.)
+
+Example log before the patch:
+
+> kern/dl.c:736: updating attributes for GOT and trampolines ("video_fb")
+> kern/efi/mm.c:927: set +rx -w on 0x13b88b000-0x13b88bfff before:rwx after:r-x
+> kern/dl.c:744: done updating module memory attributes for "video_fb"
+> kern/dl.c:639: flushing 0xe4f0 bytes at 0x13b87d000
+> kern/arm64/cache.c:42: D$ line size: 64
+> kern/arm64/cache.c:43: I$ line size: 64
+> kern/dl.c:839: module name: video_fb
+> kern/dl.c:840: init function: 0x0
+> kern/dl.c:865: Initing module video_fb
+>
+> Synchronous Exception at 0x000000013B8A76EC
+> PC 0x00013B8A76EC
+>
+>   X0 0x000000013B88B960   X1 0x0000000000000000   X2 0x000000013F93587C   X3 0x0000000000000075
+>
+>   SP 0x00000000470745C0  ELR 0x000000013B8A76EC  SPSR 0x60000205  FPSR 0x00000000
+>  ESR 0x9600004F          FAR 0x000000013B88B9D0
+>
+>  ESR : EC 0x25  IL 0x1  ISS 0x0000004F
+>
+> Data abort: Permission fault, third level
+
+Note the following:
+
+- The whole 4K page at 0x1_3B88_B000 is write-protected.
+
+- The "video_fb" module actually lives at [0x1_3B87_D000, 0x1_3B88_B4F0)
+  -- left-inclusive, right-exclusive --; that is, in the last page (at
+  0x1_3B88_B000), it only occupies the first 0x4F0 bytes.
+
+- The instruction at 0x1_3B8A_76EC faults. Not shown here, but it is a
+  store instruction, which writes to the field at offset 0x70 of the
+  structure pointed-to by the X0 register. This is the "mod->next"
+  assignment from grub_dl_init().
+
+- The faulting address is therefore (X0 + 0x70), i.e., 0x1_3B88_B9D0. This
+  is indeed the value held in the FAR register.
+
+- The faulting address 0x1_3B88_B9D0 falls in the above-noted page (at
+  0x1_3B88_B000), namely at offset 0x9D0. This is *beyond* the first 0x4F0
+  bytes that the very tail of the "video_fb" module occupies at the front
+  of that page.
+
+For now, add a self-check that reports this bug (and prevents the crash by
+skipping the write protection).
+
+Example log after the patch:
+
+> kern/dl.c:742:BUG: trying to protect pages outside of module allocation
+> ("video_fb"): module base 0x13b87d000, size 0xe4f0; tramp/GOT base
+> 0x13b88b000, size 0x1000
+
+Signed-off-by: Laszlo Ersek <lersek@redhat.com>
+---
+ grub-core/kern/dl.c | 11 ++++++++++-
+ 1 file changed, 10 insertions(+), 1 deletion(-)
+
+diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c
+index a97f4a8b13..3b66fa410e 100644
+--- a/grub-core/kern/dl.c
++++ b/grub-core/kern/dl.c
+@@ -682,7 +682,7 @@ grub_dl_set_mem_attrs (grub_dl_t mod, void *ehdr)
+ #if !defined (__i386__) && !defined (__x86_64__) && !defined(__riscv)
+   grub_size_t arch_addralign = grub_arch_dl_min_alignment ();
+   grub_addr_t tgaddr;
+-  grub_uint64_t tgsz;
++  grub_size_t tgsz;
+ #endif
+ 
+   grub_dprintf ("modules", "updating memory attributes for \"%s\"\n",
+@@ -736,6 +736,15 @@ grub_dl_set_mem_attrs (grub_dl_t mod, void *ehdr)
+       grub_dprintf ("modules",
+ 		    "updating attributes for GOT and trampolines (\"%s\")\n",
+ 		    mod->name);
++      if (tgaddr < (grub_addr_t)mod->base ||
++          tgsz > (grub_addr_t)-1 - tgaddr ||
++	  tgaddr + tgsz > (grub_addr_t)mod->base + mod->sz)
++	return grub_error (GRUB_ERR_BUG,
++			   "BUG: trying to protect pages outside of module "
++			   "allocation (\"%s\"): module base %p, size 0x%"
++			   PRIxGRUB_SIZE "; tramp/GOT base 0x%" PRIxGRUB_ADDR
++			   ", size 0x%" PRIxGRUB_SIZE,
++			   mod->name, mod->base, mod->sz, tgaddr, tgsz);
+       grub_update_mem_attrs (tgaddr, tgsz, GRUB_MEM_ATTR_R|GRUB_MEM_ATTR_X,
+ 			     GRUB_MEM_ATTR_W);
+     }

0331-grub_dl_load_segments-page-align-the-tramp-GOT-areas.patch

@@ -0,0 +1,70 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Laszlo Ersek <lersek@redhat.com>
+Date: Fri, 7 Apr 2023 16:56:09 +0200
+Subject: [PATCH] grub_dl_load_segments(): page-align the tramp/GOT areas too
+
+The tramp/GOT write-protection in grub_dl_set_mem_attrs() requires that
+the tramp/GOT areas of the module image *not* share a page with any other
+memory allocations. Page-align the tramp/GOT areas, while satisfying their
+intrinsic alignment requirements too.
+
+Fixes: 887f1d8fa976 (modules: load module sections at page-aligned addresses)
+Fixes: ad1b904d325b (nx: set page permissions for loaded modules.)
+Signed-off-by: Laszlo Ersek <lersek@redhat.com>
+---
+ grub-core/kern/dl.c | 24 ++++++++++++++++--------
+ 1 file changed, 16 insertions(+), 8 deletions(-)
+
+diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c
+index 3b66fa410e..f3cdb9e0ba 100644
+--- a/grub-core/kern/dl.c
++++ b/grub-core/kern/dl.c
+@@ -280,7 +280,9 @@ grub_dl_load_segments (grub_dl_t mod, const Elf_Ehdr *e)
+   grub_size_t tsize = 0, talign = 1, arch_addralign = 1;
+ #if !defined (__i386__) && !defined (__x86_64__) && !defined(__riscv)
+   grub_size_t tramp;
++  grub_size_t tramp_align;
+   grub_size_t got;
++  grub_size_t got_align;
+   grub_err_t err;
+ #endif
+   char *ptr;
+@@ -311,12 +313,18 @@ grub_dl_load_segments (grub_dl_t mod, const Elf_Ehdr *e)
+   err = grub_arch_dl_get_tramp_got_size (e, &tramp, &got);
+   if (err)
+     return err;
+-  tsize += ALIGN_UP (tramp, GRUB_ARCH_DL_TRAMP_ALIGN);
+-  if (talign < GRUB_ARCH_DL_TRAMP_ALIGN)
+-    talign = GRUB_ARCH_DL_TRAMP_ALIGN;
+-  tsize += ALIGN_UP (got, GRUB_ARCH_DL_GOT_ALIGN);
+-  if (talign < GRUB_ARCH_DL_GOT_ALIGN)
+-    talign = GRUB_ARCH_DL_GOT_ALIGN;
++  tramp_align = GRUB_ARCH_DL_TRAMP_ALIGN;
++  if (tramp_align < arch_addralign)
++    tramp_align = arch_addralign;
++  tsize += ALIGN_UP (tramp, tramp_align);
++  if (talign < tramp_align)
++    talign = tramp_align;
++  got_align = GRUB_ARCH_DL_GOT_ALIGN;
++  if (got_align < arch_addralign)
++    got_align = arch_addralign;
++  tsize += ALIGN_UP (got, got_align);
++  if (talign < got_align)
++    talign = got_align;
+ #endif
+ 
+ #ifdef GRUB_MACHINE_EMU
+@@ -376,11 +384,11 @@ grub_dl_load_segments (grub_dl_t mod, const Elf_Ehdr *e)
+ 	}
+     }
+ #if !defined (__i386__) && !defined (__x86_64__) && !defined(__riscv)
+-  ptr = (char *) ALIGN_UP ((grub_addr_t) ptr, GRUB_ARCH_DL_TRAMP_ALIGN);
++  ptr = (char *) ALIGN_UP ((grub_addr_t) ptr, tramp_align);
+   mod->tramp = ptr;
+   mod->trampptr = ptr;
+   ptr += tramp;
+-  ptr = (char *) ALIGN_UP ((grub_addr_t) ptr, GRUB_ARCH_DL_GOT_ALIGN);
++  ptr = (char *) ALIGN_UP ((grub_addr_t) ptr, got_align);
+   mod->got = ptr;
+   mod->gotptr = ptr;
+   ptr += got;

grub.patches

@@ -326,3 +326,6 @@ Patch0325: 0325-emu-linux-work-around-systemctl-kexec-returning.patch
 Patch0326: 0326-kern-ieee1275-init-Convert-plain-numbers-to-constant.patch
 Patch0327: 0327-kern-ieee1275-init-Extended-support-in-Vec5.patch
 Patch0328: 0328-tpm-Disable-the-tpm-verifier-if-the-TPM-device-is-no.patch
+Patch0329: 0329-grub_dl_set_mem_attrs-fix-format-string.patch
+Patch0330: 0330-grub_dl_set_mem_attrs-add-self-check-for-the-tramp-G.patch
+Patch0331: 0331-grub_dl_load_segments-page-align-the-tramp-GOT-areas.patch

grub2.spec

@@ -17,7 +17,7 @@
 Name:		grub2
 Epoch:		1
 Version:	2.06
-Release:	93%{?dist}
+Release:	94%{?dist}
 Summary:	Bootloader with support for Linux, Multiboot and more
 License:	GPLv3+
 URL:		http://www.gnu.org/software/grub/
@@ -544,6 +544,9 @@ mv ${EFI_HOME}/grub.cfg.stb ${EFI_HOME}/grub.cfg
 %endif
 
 %changelog
+* Mon Apr 10 2023 Robbie Harwood <rharwood@redhat.com> - 2.06-94
+- Fix aa64 page fault with EFI_MEMORY_ATTRIBUTE_PROTOCOL
+
 * Fri Mar 31 2023 Robbie Harwood <rharwood@redhat.com> - 2.06-93
 - Add legacy pxe core.0 (cmadams)