[PATCH resend 4/9] linux/arm: unify ARM/arm64 vs Xen PE/COFF header handling

2022-08-18 Thread Ard Biesheuvel
Xen has its own version of the image header, to account for the
additional PE/COFF header fields. Since we are adding references to
those in the shared EFI loader code, update the common definitions
and drop the Xen specific one which no longer has a purpose.

Signed-off-by: Ard Biesheuvel 
---
 grub-core/loader/arm64/linux.c| 12 +-
 grub-core/loader/arm64/xen_boot.c | 23 
 include/grub/arm/linux.h  |  6 +
 include/grub/arm64/linux.h|  4 
 include/grub/efi/efi.h|  4 +++-
 5 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index b5b559c236e0..7c0f17cf933d 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -49,8 +49,13 @@ static grub_addr_t initrd_start;
 static grub_addr_t initrd_end;
 
 grub_err_t
-grub_arch_efi_linux_check_image (struct linux_arch_kernel_header * lh)
+grub_arch_efi_linux_load_image_header (grub_file_t file,
+  struct linux_arch_kernel_header * lh)
 {
+  grub_file_seek (file, 0);
+  if (grub_file_read (file, lh, sizeof (*lh)) < (long) sizeof (*lh))
+return grub_error(GRUB_ERR_FILE_READ_ERROR, "failed to read Linux image 
header");
+
   if ((lh->code0 & 0x) != GRUB_PE32_MAGIC)
 return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
   N_("plain image kernel not supported - rebuild with 
CONFIG_(U)EFI_STUB enabled"));
@@ -301,10 +306,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ 
((unused)),
 
   kernel_size = grub_file_size (file);
 
-  if (grub_file_read (file, &lh, sizeof (lh)) < (long) sizeof (lh))
-return grub_errno;
-
-  if (grub_arch_efi_linux_check_image (&lh) != GRUB_ERR_NONE)
+  if (grub_arch_efi_linux_load_image_header (file, &lh) != GRUB_ERR_NONE)
 goto fail;
 
   grub_loader_unset();
diff --git a/grub-core/loader/arm64/xen_boot.c 
b/grub-core/loader/arm64/xen_boot.c
index 22cc25eccd9d..e5895ee78218 100644
--- a/grub-core/loader/arm64/xen_boot.c
+++ b/grub-core/loader/arm64/xen_boot.c
@@ -31,7 +31,6 @@
 #include 
 #include 
 #include 
-#include  /* required by struct xen_hypervisor_header */
 #include 
 #include 
 
@@ -65,18 +64,6 @@ enum module_type
 };
 typedef enum module_type module_type_t;
 
-struct xen_hypervisor_header
-{
-  struct linux_arm64_kernel_header efi_head;
-
-  /* This is always PE\0\0.  */
-  grub_uint8_t signature[GRUB_PE32_SIGNATURE_SIZE];
-  /* The COFF file header.  */
-  struct grub_pe32_coff_header coff_header;
-  /* The Optional header.  */
-  struct grub_pe64_optional_header optional_header;
-};
-
 struct xen_boot_binary
 {
   struct xen_boot_binary *next;
@@ -452,7 +439,7 @@ static grub_err_t
 grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ ((unused)),
 int argc, char *argv[])
 {
-  struct xen_hypervisor_header sh;
+  struct linux_arm64_kernel_header lh;
   grub_file_t file = NULL;
 
   grub_dl_ref (my_mod);
@@ -467,10 +454,7 @@ grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ 
((unused)),
   if (!file)
 goto fail;
 
-  if (grub_file_read (file, &sh, sizeof (sh)) != (long) sizeof (sh))
-goto fail;
-  if (grub_arch_efi_linux_check_image
-  ((struct linux_arch_kernel_header *) &sh) != GRUB_ERR_NONE)
+  if (grub_arch_efi_linux_load_image_header (file, &lh) != GRUB_ERR_NONE)
 goto fail;
   grub_file_seek (file, 0);
 
@@ -484,7 +468,8 @@ grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ 
((unused)),
 return grub_errno;
 
   xen_hypervisor->is_hypervisor = 1;
-  xen_hypervisor->align = (grub_size_t) sh.optional_header.section_alignment;
+  xen_hypervisor->align
+= (grub_size_t) lh.coff_image_header.optional_header.section_alignment;
 
   xen_boot_binary_load (xen_hypervisor, file, argc, argv);
   if (grub_errno == GRUB_ERR_NONE)
diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h
index bfab334dd87f..f2a2c0379795 100644
--- a/include/grub/arm/linux.h
+++ b/include/grub/arm/linux.h
@@ -22,6 +22,8 @@
 
 #include "system.h"
 
+#include 
+
 #define GRUB_LINUX_ARM_MAGIC_SIGNATURE 0x016f2818
 
 struct linux_arm_kernel_header {
@@ -32,6 +34,10 @@ struct linux_arm_kernel_header {
   grub_uint32_t end;   /* _edata */
   grub_uint32_t reserved2[3];
   grub_uint32_t hdr_offset;
+
+#if defined GRUB_MACHINE_EFI
+  struct grub_coff_image_header coff_image_header;
+#endif
 };
 
 #if defined(__arm__)
diff --git a/include/grub/arm64/linux.h b/include/grub/arm64/linux.h
index 96f1494e05a2..e00bbcfa2ff2 100644
--- a/include/grub/arm64/linux.h
+++ b/include/grub/arm64/linux.h
@@ -21,6 +21,8 @@
 
 #include 
 
+#include 
+
 #define GRUB_LINUX_ARM64_MAGIC_SIGNATURE 0x644d5241 /* 'ARM\x64' */
 
 /* From linux/Documentation/arm64/booting.txt */
@@ -36,6 +38,8 @@ struct linux_arm64_kernel_header
   grub_uint64_t res4;  /* reserved */
   grub_uint32_t magic; /* Magic number, little endian, "ARM\x64" */
   grub_uint32_t hdr_offs

[PATCH resend 3/9] arm64/linux: Remove magic number header field check

2022-08-18 Thread Ard Biesheuvel
The 'ARM\x64' magic number in the file header identifies an image as one
that implements the bare metal boot protocol, allowing the loader to
simply move the file to a suitably aligned address in memory, with
sufficient headroom for the trailing .bss segment (the required memory
size is described in the header as well).

Note of this matters for GRUB, as it only supports EFI boot. EFI does
not care about this magic number, and nor should GRUB: this prevents us
from booting other PE linux images, such as the generic EFI zboot
decompressor, which is a pure PE/COFF image, and does not implement the
bare metal boot protocol.

So drop the magic number check.

Signed-off-by: Ard Biesheuvel 
---
 grub-core/loader/arm64/linux.c | 3 ---
 include/grub/arm/linux.h   | 1 -
 include/grub/arm64/linux.h | 1 -
 3 files changed, 5 deletions(-)

diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index aed7a200b848..b5b559c236e0 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -51,9 +51,6 @@ static grub_addr_t initrd_end;
 grub_err_t
 grub_arch_efi_linux_check_image (struct linux_arch_kernel_header * lh)
 {
-  if (lh->magic != GRUB_LINUX_ARMXX_MAGIC_SIGNATURE)
-return grub_error(GRUB_ERR_BAD_OS, "invalid magic number");
-
   if ((lh->code0 & 0x) != GRUB_PE32_MAGIC)
 return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
   N_("plain image kernel not supported - rebuild with 
CONFIG_(U)EFI_STUB enabled"));
diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h
index bcd5a7eb186e..bfab334dd87f 100644
--- a/include/grub/arm/linux.h
+++ b/include/grub/arm/linux.h
@@ -35,7 +35,6 @@ struct linux_arm_kernel_header {
 };
 
 #if defined(__arm__)
-# define GRUB_LINUX_ARMXX_MAGIC_SIGNATURE GRUB_LINUX_ARM_MAGIC_SIGNATURE
 # define linux_arch_kernel_header linux_arm_kernel_header
 #endif
 
diff --git a/include/grub/arm64/linux.h b/include/grub/arm64/linux.h
index 7e22b4ab6990..96f1494e05a2 100644
--- a/include/grub/arm64/linux.h
+++ b/include/grub/arm64/linux.h
@@ -39,7 +39,6 @@ struct linux_arm64_kernel_header
 };
 
 #if defined(__aarch64__)
-# define GRUB_LINUX_ARMXX_MAGIC_SIGNATURE GRUB_LINUX_ARM64_MAGIC_SIGNATURE
 # define linux_arch_kernel_header linux_arm64_kernel_header
 #endif
 
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH resend 2/9] efi: move MS-DOS stub out of generic PE header definition

2022-08-18 Thread Ard Biesheuvel
The PE/COFF spec permits the COFF signature and file header to appear
anywhere in the file, and the actual offset is recorded in 4 byte
little endian field at offset 0x3c of the image.

When GRUB is emitted as a PE/COFF binary, we reuse the 128 byte MS-DOS
stub (even for non-x86 architectures), putting the COFF signature and
file header at offset 0x80. However, other PE/COFF images may use
different values, and non-x86 Linux kernels use an offset of 0x40
instead.

So let's get rid of the grub_pe32_header struct from pe32.h, given that
it does not represent anything defined by the PE/COFF spec. Instead,
use the GRUB_PE32_MSDOS_STUB_SIZE macro explicitly to reference the
COFF header in the only place in the code where we rely on this.

The remaining fields are moved into a struct grub_coff_image_header,
which we will use later to access COFF header fields of arbitrary
images (and which may therefore appear at different offsets)

Signed-off-by: Ard Biesheuvel 
---
 grub-core/kern/efi/efi.c | 5 +++--
 include/grub/efi/pe32.h  | 5 +
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index e8a976a22f15..8bef81663853 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -302,7 +302,7 @@ grub_addr_t
 grub_efi_modules_addr (void)
 {
   grub_efi_loaded_image_t *image;
-  struct grub_pe32_header *header;
+  struct grub_coff_image_header *header;
   struct grub_pe32_coff_header *coff_header;
   struct grub_pe32_section_table *sections;
   struct grub_pe32_section_table *section;
@@ -313,7 +313,8 @@ grub_efi_modules_addr (void)
   if (! image)
 return 0;
 
-  header = image->image_base;
+  header = (struct grub_coff_image_header *) ((char *) image->image_base
+ + GRUB_PE32_MSDOS_STUB_SIZE);
   coff_header = &(header->coff_header);
   sections
 = (struct grub_pe32_section_table *) ((char *) coff_header
diff --git a/include/grub/efi/pe32.h b/include/grub/efi/pe32.h
index 0ed8781f0376..a2da4b318c85 100644
--- a/include/grub/efi/pe32.h
+++ b/include/grub/efi/pe32.h
@@ -254,11 +254,8 @@ struct grub_pe32_section_table
 
 #define GRUB_PE32_SIGNATURE_SIZE 4
 
-struct grub_pe32_header
+struct grub_coff_image_header
 {
-  /* This should be filled in with GRUB_PE32_MSDOS_STUB.  */
-  grub_uint8_t msdos_stub[GRUB_PE32_MSDOS_STUB_SIZE];
-
   /* This is always PE\0\0.  */
   char signature[GRUB_PE32_SIGNATURE_SIZE];
 
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH resend 8/9] efi: implement LoadFile2 initrd loading protocol for Linux

2022-08-18 Thread Ard Biesheuvel
Recent Linux kernels will invoke the LoadFile2 protocol installed on
a well-known vendor media path to load the initrd if it is exposed by
the firmware. Using this method is preferred for two reasons:
- the Linux kernel is in charge of allocating the memory, and so it can
  implement any placement policy it wants (given that these tend to
  change between kernel versions),
- it is no longer necessary to modify the device tree provided by the
  firmware.

So let's install this protocol when handling the 'initrd' command if
such a recent kernel was detected (based on the PE/COFF image version),
and defer loading the initrd contents until the point where the kernel
invokes the LoadFile2 protocol.

Signed-off-by: Ard Biesheuvel 
---
 grub-core/loader/arm64/linux.c | 129 +++-
 1 file changed, 128 insertions(+), 1 deletion(-)

diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index 56ba8d0a6ea3..82c7558b4c4c 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -48,6 +48,48 @@ static grub_uint32_t cmdline_size;
 static grub_addr_t initrd_start;
 static grub_addr_t initrd_end;
 
+static grub_efi_guid_t load_file2_guid = GRUB_EFI_LOAD_FILE2_PROTOCOL_GUID;
+static grub_efi_guid_t device_path_guid = GRUB_EFI_DEVICE_PATH_GUID;
+static struct grub_linux_initrd_context initrd_ctx = { 0, 0, 0 };
+static grub_efi_handle_t initrd_lf2_handle;
+static int initrd_use_loadfile2;
+
+struct initrd_media_device_path {
+  grub_efi_vendor_media_device_path_t  vendor;
+  grub_efi_device_path_t   end;
+} GRUB_PACKED;
+
+#define LINUX_EFI_INITRD_MEDIA_GUID  \
+  { 0x5568e427, 0x68fc, 0x4f3d, \
+{ 0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68 } \
+  }
+
+static struct initrd_media_device_path initrd_lf2_device_path = {
+  {
+{
+  GRUB_EFI_MEDIA_DEVICE_PATH_TYPE,
+  GRUB_EFI_VENDOR_MEDIA_DEVICE_PATH_SUBTYPE,
+  sizeof(grub_efi_vendor_media_device_path_t),
+},
+LINUX_EFI_INITRD_MEDIA_GUID
+  }, {
+GRUB_EFI_END_DEVICE_PATH_TYPE,
+GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE,
+sizeof(grub_efi_device_path_t)
+  }
+};
+
+static grub_efi_status_t
+grub_efi_initrd_load_file2(grub_efi_load_file2_t *this,
+   grub_efi_device_path_t *device_path,
+   grub_efi_boolean_t boot_policy,
+   grub_efi_uintn_t *buffer_size,
+   void *buffer);
+
+static grub_efi_load_file2_t initrd_lf2 = {
+  grub_efi_initrd_load_file2
+};
+
 grub_err_t
 grub_arch_efi_linux_load_image_header (grub_file_t file,
   struct linux_arch_kernel_header * lh)
@@ -78,6 +120,18 @@ grub_arch_efi_linux_load_image_header (grub_file_t file,
return grub_error(GRUB_ERR_FILE_READ_ERROR, "failed to read COFF image 
header");
 }
 
+  /*
+   * Linux kernels built for any architecture are guaranteed to support the
+   * LoadFile2 based initrd loading protocol if the image version is >= 1.
+   */
+  if (lh->coff_image_header.optional_header.major_image_version >= 1)
+initrd_use_loadfile2 = 1;
+   else
+initrd_use_loadfile2 = 0;
+
+  grub_dprintf ("linux", "LoadFile2 initrd loading %sabled\n",
+initrd_use_loadfile2 ? "en" : "dis");
+
   return GRUB_ERR_NONE;
 }
 
@@ -197,6 +251,8 @@ grub_linux_boot (void)
 static grub_err_t
 grub_linux_unload (void)
 {
+  grub_efi_boot_services_t *b;
+
   grub_dl_unref (my_mod);
   loaded = 0;
   if (initrd_start)
@@ -208,6 +264,19 @@ grub_linux_unload (void)
 grub_efi_free_pages ((grub_addr_t) kernel_addr,
 GRUB_EFI_BYTES_TO_PAGES (kernel_size));
   grub_fdt_unload ();
+
+  if (initrd_lf2_handle)
+{
+  b = grub_efi_system_table->boot_services;
+  b->uninstall_multiple_protocol_interfaces (initrd_lf2_handle,
+ &load_file2_guid,
+ &initrd_lf2,
+ &device_path_guid,
+ &initrd_lf2_device_path,
+ NULL);
+  initrd_lf2_handle = NULL;
+  initrd_use_loadfile2 = 0;
+}
   return GRUB_ERR_NONE;
 }
 
@@ -247,13 +316,50 @@ allocate_initrd_mem (int initrd_pages)
   GRUB_EFI_LOADER_DATA);
 }
 
+static grub_efi_status_t
+grub_efi_initrd_load_file2(grub_efi_load_file2_t *this,
+  grub_efi_device_path_t *device_path,
+  grub_efi_boolean_t boot_policy,
+  grub_efi_uintn_t *buffer_size,
+  void *buffer)
+{
+  grub_efi_status_t status = GRUB_EFI_SUCCESS;
+  grub_efi_uintn_t initrd_size;
+
+  if (!this || this != &initrd_lf2 || !buffer_size)
+return GRUB_EFI_INVALID_PARAMETER;
+
+  if (device_path->type != GRUB_EFI_END_DEVICE_PATH_TYPE ||
+  device_path->subtype != GRUB_EFI_EN

[PATCH resend 1/9] loader: drop argv[] argument in grub_initrd_load()

2022-08-18 Thread Ard Biesheuvel
From: Nikita Ermakov 

In the case of an error grub_initrd_load() uses argv[] to print the
filename that caused the error. It is also possible to obtain the
filename from the file handles and there is no need to duplicate that
information in argv[], so let's drop it.

Signed-off-by: Nikita Ermakov 
Signed-off-by: Ard Biesheuvel 
---
 grub-core/loader/arm/linux.c  | 2 +-
 grub-core/loader/arm64/linux.c| 2 +-
 grub-core/loader/i386/linux.c | 2 +-
 grub-core/loader/i386/pc/linux.c  | 2 +-
 grub-core/loader/i386/xen.c   | 3 +--
 grub-core/loader/ia64/efi/linux.c | 2 +-
 grub-core/loader/linux.c  | 4 ++--
 grub-core/loader/mips/linux.c | 2 +-
 grub-core/loader/powerpc/ieee1275/linux.c | 2 +-
 grub-core/loader/sparc64/ieee1275/linux.c | 2 +-
 include/grub/linux.h  | 2 +-
 11 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/grub-core/loader/arm/linux.c b/grub-core/loader/arm/linux.c
index 30b366601c39..f00b538ebad0 100644
--- a/grub-core/loader/arm/linux.c
+++ b/grub-core/loader/arm/linux.c
@@ -422,7 +422,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
((unused)),
   grub_dprintf ("loader", "Loading initrd to 0x%08x\n",
(grub_addr_t) initrd_start);
 
-  if (grub_initrd_load (&initrd_ctx, argv, (void *) initrd_start))
+  if (grub_initrd_load (&initrd_ctx, (void *) initrd_start))
 goto fail;
 
   initrd_end = initrd_start + size;
diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index ef3e9f9444ca..aed7a200b848 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -266,7 +266,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
((unused)),
   goto fail;
 }
 
-  if (grub_initrd_load (&initrd_ctx, argv, initrd_mem))
+  if (grub_initrd_load (&initrd_ctx, initrd_mem))
 goto fail;
 
   initrd_start = (grub_addr_t) initrd_mem;
diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
index c5984d4b27e9..edd6c2bb1d23 100644
--- a/grub-core/loader/i386/linux.c
+++ b/grub-core/loader/i386/linux.c
@@ -1107,7 +1107,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
((unused)),
 initrd_mem_target = get_physical_target_address (ch);
   }
 
-  if (grub_initrd_load (&initrd_ctx, argv, initrd_mem))
+  if (grub_initrd_load (&initrd_ctx, initrd_mem))
 goto fail;
 
   grub_dprintf ("linux", "Initrd, addr=0x%x, size=0x%x\n",
diff --git a/grub-core/loader/i386/pc/linux.c b/grub-core/loader/i386/pc/linux.c
index 7b89d431051e..4adeee9ae001 100644
--- a/grub-core/loader/i386/pc/linux.c
+++ b/grub-core/loader/i386/pc/linux.c
@@ -462,7 +462,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
((unused)),
 initrd_addr = get_physical_target_address (ch);
   }
 
-  if (grub_initrd_load (&initrd_ctx, argv, initrd_chunk))
+  if (grub_initrd_load (&initrd_ctx, initrd_chunk))
 goto fail;
 
   lh->ramdisk_image = initrd_addr;
diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
index cd24874ca324..3b856e842709 100644
--- a/grub-core/loader/i386/xen.c
+++ b/grub-core/loader/i386/xen.c
@@ -809,8 +809,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
((unused)),
   if (err)
goto fail;
 
-  if (grub_initrd_load (&initrd_ctx, argv,
-   get_virtual_current_address (ch)))
+  if (grub_initrd_load (&initrd_ctx, get_virtual_current_address (ch)))
goto fail;
 }
 
diff --git a/grub-core/loader/ia64/efi/linux.c 
b/grub-core/loader/ia64/efi/linux.c
index 41266109e03c..fb9b961f71a2 100644
--- a/grub-core/loader/ia64/efi/linux.c
+++ b/grub-core/loader/ia64/efi/linux.c
@@ -563,7 +563,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
((unused)),
   grub_dprintf ("linux", "[addr=0x%lx, size=0x%lx]\n",
(grub_uint64_t) initrd_mem, initrd_size);
 
-  if (grub_initrd_load (&initrd_ctx, argv, initrd_mem))
+  if (grub_initrd_load (&initrd_ctx, initrd_mem))
 goto fail;
  fail:
   grub_initrd_close (&initrd_ctx);
diff --git a/grub-core/loader/linux.c b/grub-core/loader/linux.c
index ade9b2fef470..830360172c3d 100644
--- a/grub-core/loader/linux.c
+++ b/grub-core/loader/linux.c
@@ -271,7 +271,7 @@ grub_initrd_close (struct grub_linux_initrd_context 
*initrd_ctx)
 
 grub_err_t
 grub_initrd_load (struct grub_linux_initrd_context *initrd_ctx,
- char *argv[], void *target)
+ void *target)
 {
   grub_uint8_t *ptr = target;
   int i;
@@ -317,7 +317,7 @@ grub_initrd_load (struct grub_linux_initrd_context 
*initrd_ctx,
{
  if (!grub_errno)
grub_error (GRUB_ERR_FILE_READ_ERROR, N_("premature end of file 
%s"),
-   argv[i]);
+   initrd_ctx->components[i].file->name);
  grub_initrd_close (initrd_ctx);
  return grub_errno;
}
diff --git a/grub-core/loader/mips/linux.c b/grub-core/loader/mips/linux.c
i

[PATCH resend 6/9] efi: add definition of LoadFile2 protocol

2022-08-18 Thread Ard Biesheuvel
Incorporate the EFI_LOAD_FILE2_PROTOCOL GUID and C types from the
UEFI spec.

Reviewed-by: Heinrich Schuchardt 
Signed-off-by: Ard Biesheuvel 
---
 grub-core/commands/efi/lsefi.c |  1 +
 include/grub/efi/api.h | 15 +++
 2 files changed, 16 insertions(+)

diff --git a/grub-core/commands/efi/lsefi.c b/grub-core/commands/efi/lsefi.c
index f46ba3b49384..c304d25ccdd6 100644
--- a/grub-core/commands/efi/lsefi.c
+++ b/grub-core/commands/efi/lsefi.c
@@ -55,6 +55,7 @@ struct known_protocol
 { GRUB_EFI_ABSOLUTE_POINTER_PROTOCOL_GUID, "absolute pointer" },
 { GRUB_EFI_DRIVER_BINDING_PROTOCOL_GUID, "EFI driver binding" },
 { GRUB_EFI_LOAD_FILE_PROTOCOL_GUID, "load file" },
+{ GRUB_EFI_LOAD_FILE2_PROTOCOL_GUID, "load file2" },
 { GRUB_EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID, "simple FS" },
 { GRUB_EFI_TAPE_IO_PROTOCOL_GUID, "tape I/O" },
 { GRUB_EFI_UNICODE_COLLATION_PROTOCOL_GUID, "unicode collation" },
diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
index 1ef4046225cb..1077826a3ca1 100644
--- a/include/grub/efi/api.h
+++ b/include/grub/efi/api.h
@@ -149,6 +149,11 @@
 { 0x8E, 0x3F, 0x00, 0xA0, 0xC9, 0x69, 0x72, 0x3B } \
   }
 
+#define GRUB_EFI_LOAD_FILE2_PROTOCOL_GUID \
+  { 0x4006c0c1, 0xfcb3, 0x403e, \
+{ 0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d } \
+  }
+
 #define GRUB_EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID \
   { 0x0964e5b22, 0x6459, 0x11d2, \
 { 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b } \
@@ -1749,6 +1754,16 @@ struct grub_efi_rng_protocol
 };
 typedef struct grub_efi_rng_protocol grub_efi_rng_protocol_t;
 
+struct grub_efi_load_file2
+{
+  grub_efi_status_t (*load_file)(struct grub_efi_load_file2 *this,
+grub_efi_device_path_t *file_path,
+grub_efi_boolean_t boot_policy,
+grub_efi_uintn_t *buffer_size,
+void *buffer);
+};
+typedef struct grub_efi_load_file2 grub_efi_load_file2_t;
+
 #if (GRUB_TARGET_SIZEOF_VOID_P == 4) || defined (__ia64__) \
   || defined (__aarch64__) || defined (__MINGW64__) || defined (__CYGWIN__) \
   || defined(__riscv)
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH resend 0/9] linux: implement LoadFile2 initrd loading

2022-08-18 Thread Ard Biesheuvel
On Thu, 18 Aug 2022 at 10:55, Ard Biesheuvel  wrote:
>
> This implements the LoadFile2 initrd loading protocol, which is
> essentially a callback interface into the bootloader to load the initrd
> data into a caller provided buffer. This means the bootloader no longer
> has to contain any policy regarding where to load the initrd (which
> differs between architectures and kernel versions) and no longer has to
> manipulate arch specific data structures such as DT or struct bootparams
> to inform the OS where the initrd resides in memory. This is especially
> relevant for the upcoming LoongArch support, which does not use either
> DT or struct bootparams, and would have to rely on the initrd= command
> line interface, which is deprecated and of limited utility [0].
>
> Sample output from booting a recent Linux/arm64 kernel:
>
>   grub> insmod part_msdos
>   grub> linux (hd0,msdos1)/Image
>   grub> initrd (hd0,msdos1)/initrd.img
>   grub> boot
>   EFI stub: Booting Linux Kernel...
>   EFI stub: EFI_RNG_PROTOCOL unavailable, KASLR will be disabled
>   EFI stub: Generating empty DTB
>   EFI stub: Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path
>   EFI stub: Exiting boot services and installing virtual address map...
>   [0.00] Booting Linux on physical CPU 0x00 [0x411fd070]
>
> This is mostly a resend of my original v2 [1], although I did
> cherry-pick Nikita's version of the first patch, which incorporates
> Heinrich's suggestion to simply drop the argv[] argument from
> grub_initrd_load(). I also included the patch I sent out the other day
> to remove the pointless header magic number check, and included a fix
> for the PXE boot issue reported by dann [2].
>
> [0] The initrd= command line loader can only access files that reside on
> the same volume as the loaded image, which means GRUB would have to
> present this volume abstraction in order to serve the initrd file.
> Another reason why this method is problematic is generic EFI zboot,
> which is being added to Linux, and which calls loadimage on another,
> embedded PE/COFF image which would also need to expose this volume
> abstraction.
>
> [1] https://lists.gnu.org/archive/html/grub-devel/2020-10/msg00124.html
> [2] https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00055.html
>
> Cc: grub-devel@gnu.org

When cc'ing to this series, please note that i included an incorrect
grub-devel@ address too - please remove.

> Cc: Daniel Kiper 
> Cc: Nikita Ermakov 
> Cc: Atish Patra 
> Cc: Huacai Chen 
> Cc: Heinrich Schuchardt 
> Cc: dann frazier 
> Cc: Julian Andres Klode 
>
> Ard Biesheuvel (8):
>   efi: move MS-DOS stub out of generic PE header definition
>   arm64/linux: Remove magic number header field check
>   linux/arm: unify ARM/arm64 vs Xen PE/COFF header handling
>   linux/arm: account for COFF headers appearing at unexpected offsets
>   efi: add definition of LoadFile2 protocol
>   efi/efinet: Don't close connections at fini_hw() time
>   efi: implement LoadFile2 initrd loading protocol for Linux
>   linux: ignore FDT unless we need to modify it
>
> Nikita Ermakov (1):
>   loader: drop argv[] argument in grub_initrd_load()
>
>  grub-core/commands/efi/lsefi.c|   1 +
>  grub-core/kern/efi/efi.c  |   5 +-
>  grub-core/loader/arm/linux.c  |   2 +-
>  grub-core/loader/arm64/linux.c| 181 +---
>  grub-core/loader/arm64/xen_boot.c |  23 +--
>  grub-core/loader/efi/fdt.c|   7 +-
>  grub-core/loader/i386/linux.c |   2 +-
>  grub-core/loader/i386/pc/linux.c  |   2 +-
>  grub-core/loader/i386/xen.c   |   3 +-
>  grub-core/loader/ia64/efi/linux.c |   2 +-
>  grub-core/loader/linux.c  |   4 +-
>  grub-core/loader/mips/linux.c |   2 +-
>  grub-core/loader/powerpc/ieee1275/linux.c |   2 +-
>  grub-core/loader/sparc64/ieee1275/linux.c |   2 +-
>  grub-core/net/drivers/efi/efinet.c|  10 +-
>  grub-core/net/net.c   |   2 +-
>  include/grub/arm/linux.h  |   7 +-
>  include/grub/arm64/linux.h|   5 +-
>  include/grub/efi/api.h|  15 ++
>  include/grub/efi/efi.h|   4 +-
>  include/grub/efi/pe32.h   |   5 +-
>  include/grub/linux.h  |   2 +-
>  include/grub/net.h|   3 +-
>  23 files changed, 226 insertions(+), 65 deletions(-)
>
> --
> 2.35.1
>

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH resend 0/9] linux: implement LoadFile2 initrd loading

2022-08-18 Thread Ard Biesheuvel
This implements the LoadFile2 initrd loading protocol, which is
essentially a callback interface into the bootloader to load the initrd
data into a caller provided buffer. This means the bootloader no longer
has to contain any policy regarding where to load the initrd (which
differs between architectures and kernel versions) and no longer has to
manipulate arch specific data structures such as DT or struct bootparams
to inform the OS where the initrd resides in memory. This is especially
relevant for the upcoming LoongArch support, which does not use either
DT or struct bootparams, and would have to rely on the initrd= command
line interface, which is deprecated and of limited utility [0].

Sample output from booting a recent Linux/arm64 kernel:

  grub> insmod part_msdos
  grub> linux (hd0,msdos1)/Image
  grub> initrd (hd0,msdos1)/initrd.img
  grub> boot
  EFI stub: Booting Linux Kernel...
  EFI stub: EFI_RNG_PROTOCOL unavailable, KASLR will be disabled
  EFI stub: Generating empty DTB
  EFI stub: Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path
  EFI stub: Exiting boot services and installing virtual address map...
  [0.00] Booting Linux on physical CPU 0x00 [0x411fd070]

This is mostly a resend of my original v2 [1], although I did
cherry-pick Nikita's version of the first patch, which incorporates
Heinrich's suggestion to simply drop the argv[] argument from
grub_initrd_load(). I also included the patch I sent out the other day
to remove the pointless header magic number check, and included a fix
for the PXE boot issue reported by dann [2].

[0] The initrd= command line loader can only access files that reside on
the same volume as the loaded image, which means GRUB would have to
present this volume abstraction in order to serve the initrd file.
Another reason why this method is problematic is generic EFI zboot,
which is being added to Linux, and which calls loadimage on another,
embedded PE/COFF image which would also need to expose this volume
abstraction.

[1] https://lists.gnu.org/archive/html/grub-devel/2020-10/msg00124.html
[2] https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00055.html

Cc: grub-devel@gnu.org
Cc: Daniel Kiper 
Cc: Nikita Ermakov 
Cc: Atish Patra 
Cc: Huacai Chen 
Cc: Heinrich Schuchardt 
Cc: dann frazier 
Cc: Julian Andres Klode 

Ard Biesheuvel (8):
  efi: move MS-DOS stub out of generic PE header definition
  arm64/linux: Remove magic number header field check
  linux/arm: unify ARM/arm64 vs Xen PE/COFF header handling
  linux/arm: account for COFF headers appearing at unexpected offsets
  efi: add definition of LoadFile2 protocol
  efi/efinet: Don't close connections at fini_hw() time
  efi: implement LoadFile2 initrd loading protocol for Linux
  linux: ignore FDT unless we need to modify it

Nikita Ermakov (1):
  loader: drop argv[] argument in grub_initrd_load()

 grub-core/commands/efi/lsefi.c|   1 +
 grub-core/kern/efi/efi.c  |   5 +-
 grub-core/loader/arm/linux.c  |   2 +-
 grub-core/loader/arm64/linux.c| 181 +---
 grub-core/loader/arm64/xen_boot.c |  23 +--
 grub-core/loader/efi/fdt.c|   7 +-
 grub-core/loader/i386/linux.c |   2 +-
 grub-core/loader/i386/pc/linux.c  |   2 +-
 grub-core/loader/i386/xen.c   |   3 +-
 grub-core/loader/ia64/efi/linux.c |   2 +-
 grub-core/loader/linux.c  |   4 +-
 grub-core/loader/mips/linux.c |   2 +-
 grub-core/loader/powerpc/ieee1275/linux.c |   2 +-
 grub-core/loader/sparc64/ieee1275/linux.c |   2 +-
 grub-core/net/drivers/efi/efinet.c|  10 +-
 grub-core/net/net.c   |   2 +-
 include/grub/arm/linux.h  |   7 +-
 include/grub/arm64/linux.h|   5 +-
 include/grub/efi/api.h|  15 ++
 include/grub/efi/efi.h|   4 +-
 include/grub/efi/pe32.h   |   5 +-
 include/grub/linux.h  |   2 +-
 include/grub/net.h|   3 +-
 23 files changed, 226 insertions(+), 65 deletions(-)

-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH resend 5/9] linux/arm: account for COFF headers appearing at unexpected offsets

2022-08-18 Thread Ard Biesheuvel
The way we load the Linux and PE/COFF image headers depends on a fixed
placement of the COFF header at offset 0x40 into the file. This is a
reasonable default, given that this is where Linux emits it today.
However, in order to comply with the PE/COFF spec, which allows this
header to appear anywhere in the file, let's ensure that we read the
header from where it actually appears in the file if it is not located
at offset 0x40.

Signed-off-by: Ard Biesheuvel 
---
 grub-core/loader/arm64/linux.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index 7c0f17cf933d..56ba8d0a6ea3 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -63,6 +63,21 @@ grub_arch_efi_linux_load_image_header (grub_file_t file,
   grub_dprintf ("linux", "UEFI stub kernel:\n");
   grub_dprintf ("linux", "PE/COFF header @ %08x\n", lh->hdr_offset);
 
+  /*
+   * The PE/COFF spec permits the COFF header to appear anywhere in the file, 
so
+   * we need to double check whether it was where we expected it, and if not, 
we
+   * must load it from the correct offset into the coff_image_header field of
+   * struct linux_arch_kernel_header.
+   */
+  if ((grub_uint8_t *) lh + lh->hdr_offset != (grub_uint8_t *) 
&lh->coff_image_header)
+{
+  grub_file_seek (file, lh->hdr_offset);
+
+  if (grub_file_read (file, &lh->coff_image_header, sizeof(struct 
grub_coff_image_header))
+ != sizeof(struct grub_coff_image_header))
+   return grub_error(GRUB_ERR_FILE_READ_ERROR, "failed to read COFF image 
header");
+}
+
   return GRUB_ERR_NONE;
 }
 
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH resend 9/9] linux: ignore FDT unless we need to modify it

2022-08-18 Thread Ard Biesheuvel
Now that we implemented supported for the LoadFile2 protocol for initrd
loading, there is no longer a need to pass the initrd parameters via
the device tree. This means there is no longer a reason to update the
device tree in the first place, and so we can ignore it entirely.

The only remaining reason to deal with the devicetree is if we are
using the 'devicetree' command to load one from disk, so tweak the
logic in grub_fdt_install() to take that into account.

Signed-off-by: Ard Biesheuvel 
Reviewed-by: Leif Lindholm 
---
 grub-core/loader/arm64/linux.c | 22 ++--
 grub-core/loader/efi/fdt.c |  7 +--
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index 82c7558b4c4c..68ed1502c68a 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -142,21 +142,21 @@ finalize_params_linux (void)
 
   void *fdt;
 
-  fdt = grub_fdt_load (GRUB_EFI_LINUX_FDT_EXTRA_SPACE);
+  /* Set initrd info */
+  if (initrd_start && initrd_end > initrd_start)
+{
+  fdt = grub_fdt_load (GRUB_EFI_LINUX_FDT_EXTRA_SPACE);
 
-  if (!fdt)
-goto failure;
+  if (!fdt)
+   goto failure;
 
-  node = grub_fdt_find_subnode (fdt, 0, "chosen");
-  if (node < 0)
-node = grub_fdt_add_subnode (fdt, 0, "chosen");
+  node = grub_fdt_find_subnode (fdt, 0, "chosen");
+  if (node < 0)
+   node = grub_fdt_add_subnode (fdt, 0, "chosen");
 
-  if (node < 1)
-goto failure;
+  if (node < 1)
+   goto failure;
 
-  /* Set initrd info */
-  if (initrd_start && initrd_end > initrd_start)
-{
   grub_dprintf ("linux", "Initrd @ %p-%p\n",
(void *) initrd_start, (void *) initrd_end);
 
diff --git a/grub-core/loader/efi/fdt.c b/grub-core/loader/efi/fdt.c
index c86f283d756b..771d455c7319 100644
--- a/grub-core/loader/efi/fdt.c
+++ b/grub-core/loader/efi/fdt.c
@@ -89,13 +89,16 @@ grub_fdt_install (void)
   grub_efi_guid_t fdt_guid = GRUB_EFI_DEVICE_TREE_GUID;
   grub_efi_status_t status;
 
+  if (!fdt && !loaded_fdt)
+return GRUB_ERR_NONE;
+
   b = grub_efi_system_table->boot_services;
-  status = b->install_configuration_table (&fdt_guid, fdt);
+  status = b->install_configuration_table (&fdt_guid, fdt ?: loaded_fdt);
   if (status != GRUB_EFI_SUCCESS)
 return grub_error (GRUB_ERR_IO, "failed to install FDT");
 
   grub_dprintf ("fdt", "Installed/updated FDT configuration table @ %p\n",
-   fdt);
+   fdt ?: loaded_fdt);
   return GRUB_ERR_NONE;
 }
 
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH resend 7/9] efi/efinet: Don't close connections at fini_hw() time

2022-08-18 Thread Ard Biesheuvel
When GRUB runs on top of EFI firmware, it only has access to block and
network device abstractions exposed by the firmware, and it is up to the
firmware to quiesce the underlying hardware when handing over to the OS.

This is especially important for network devices, to prevent incoming
packets from being DMA'd straight into memory after the OS has taken
over but before it has managed to reconfigure the network hardware.

GRUB handles this by means of the grub_net_fini_hw() preboot hook, which
is executed before calling into the booted image. This means that all
network devices disappear or become inoperable before the EFI stub
executes on EFI targeted builds. This is problematic as it prevents the
EFI stub from calling back into GRUB provided protocols such as
LoadFile2 for the initrd, which we will provide in a subsequent patch.

So add a flag that indicates to the network core that EFI network
devices should not be closed when grub_net_fini_hw() is called.

Signed-off-by: Ard Biesheuvel 
---
 grub-core/net/drivers/efi/efinet.c | 10 +-
 grub-core/net/net.c|  2 +-
 include/grub/net.h |  3 ++-
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/grub-core/net/drivers/efi/efinet.c 
b/grub-core/net/drivers/efi/efinet.c
index 73343d26d9e1..5adf5f40f492 100644
--- a/grub-core/net/drivers/efi/efinet.c
+++ b/grub-core/net/drivers/efi/efinet.c
@@ -320,7 +320,15 @@ grub_efinet_findcards (void)
 
   card->name = grub_xasprintf ("efinet%d", i++);
   card->driver = &efidriver;
-  card->flags = 0;
+  /*
+   * EFI network devices are abstract SNP protocol instances, and the
+   * firmware is in charge of ensuring that they will be torn down when the
+   * OS loader hands off to the OS proper. Closing them as part of the
+   * preboot cleanup is therefore unnecessary, and undesirable, as it
+   * prevents us from using the network connection in a protocal callback
+   * such as LoadFile2 for initrd loading.
+   */
+  card->flags = GRUB_NET_CARD_NO_CLOSE_ON_FINI_HW;
   card->default_address.type = GRUB_NET_LINK_LEVEL_PROTOCOL_ETHERNET;
   grub_memcpy (card->default_address.mac,
   net->mode->current_address,
diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index 064e7114e012..7046dc57890a 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -1787,7 +1787,7 @@ grub_net_fini_hw (int noreturn __attribute__ ((unused)))
 {
   struct grub_net_card *card;
   FOR_NET_CARDS (card)
-if (card->opened)
+if (card->opened && !(card->flags & GRUB_NET_CARD_NO_CLOSE_ON_FINI_HW))
   {
if (card->driver->close)
  card->driver->close (card);
diff --git a/include/grub/net.h b/include/grub/net.h
index a64a04cc80b1..79cba357ae6a 100644
--- a/include/grub/net.h
+++ b/include/grub/net.h
@@ -64,7 +64,8 @@ typedef enum grub_net_interface_flags
 typedef enum grub_net_card_flags
   {
 GRUB_NET_CARD_HWADDRESS_IMMUTABLE = 1,
-GRUB_NET_CARD_NO_MANUAL_INTERFACES = 2
+GRUB_NET_CARD_NO_MANUAL_INTERFACES = 2,
+GRUB_NET_CARD_NO_CLOSE_ON_FINI_HW = 4
   } grub_net_card_flags_t;
 
 struct grub_net_card;
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3 0/8] linux: implement LoadFile2 initrd loading

2022-08-18 Thread Ard Biesheuvel
This implements the LoadFile2 initrd loading protocol, which is
essentially a callback interface into the bootloader to load the initrd
data into a caller provided buffer. This means the bootloader no longer
has to contain any policy regarding where to load the initrd (which
differs between architectures and kernel versions) and no longer has to
manipulate arch specific data structures such as DT or struct bootparams
to inform the OS where the initrd resides in memory. This is especially
relevant for the upcoming LoongArch support, which does not use either
DT or struct bootparams, and would have to rely on the initrd= command
line interface, which is deprecated and of limited utility [0].

Sample output from booting a recent Linux/arm64 kernel:

  grub> insmod part_msdos
  grub> linux (hd0,msdos1)/Image
  grub> initrd (hd0,msdos1)/initrd.img
  grub> boot
  EFI stub: Booting Linux Kernel...
  EFI stub: EFI_RNG_PROTOCOL unavailable, KASLR will be disabled
  EFI stub: Generating empty DTB
  EFI stub: Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path
  EFI stub: Exiting boot services and installing virtual address map...
  [0.00] Booting Linux on physical CPU 0x00 [0x411fd070]

This is mostly a resend of my original v2 [1], although I did
cherry-pick Nikita's version of the first patch, which incorporates
Heinrich's suggestion to simply drop the argv[] argument from
grub_initrd_load(). I also included the patch I sent out the other day
to remove the pointless header magic number check, and included a fix
for the PXE boot issue reported by dann [2].

Changes since v2:
- incorporate some ancient feedback from Daniel that I never saw until
  today. (this is why I am sending two versions of the same series on
  the same day - apologies for the spam)

[0] The initrd= command line loader can only access files that reside on
the same volume as the loaded image, which means GRUB would have to
present this volume abstraction in order to serve the initrd file.
Another reason why this method is problematic is generic EFI zboot,
which is being added to Linux, and which calls loadimage on another,
embedded PE/COFF image which would also need to expose this volume
abstraction.

[1] https://lists.gnu.org/archive/html/grub-devel/2020-10/msg00124.html
[2] https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00055.html

Cc: Daniel Kiper 
Cc: Leif Lindholm 
Cc: Nikita Ermakov 
Cc: Atish Patra 
Cc: Huacai Chen 
Cc: Heinrich Schuchardt 
Cc: dann frazier 
Cc: Julian Andres Klode 

Ard Biesheuvel (7):
  efi: move MS-DOS stub out of generic PE header definition
  arm64/linux: Remove magic number header field check
  linux/arm: unify ARM/arm64 vs Xen PE/COFF header handling
  linux/arm: account for COFF headers appearing at unexpected offsets
  efi/efinet: Don't close connections at fini_hw() time
  efi: implement LoadFile2 initrd loading protocol for Linux
  linux: ignore FDT unless we need to modify it

Nikita Ermakov (1):
  loader: drop argv[] argument in grub_initrd_load()

 grub-core/commands/efi/lsefi.c|   1 +
 grub-core/kern/efi/efi.c  |   5 +-
 grub-core/loader/arm/linux.c  |   2 +-
 grub-core/loader/arm64/linux.c| 175 +---
 grub-core/loader/arm64/xen_boot.c |  23 +--
 grub-core/loader/efi/fdt.c|   7 +-
 grub-core/loader/i386/linux.c |   2 +-
 grub-core/loader/i386/pc/linux.c  |   2 +-
 grub-core/loader/i386/xen.c   |   3 +-
 grub-core/loader/ia64/efi/linux.c |   2 +-
 grub-core/loader/linux.c  |   4 +-
 grub-core/loader/mips/linux.c |   2 +-
 grub-core/loader/powerpc/ieee1275/linux.c |   2 +-
 grub-core/loader/sparc64/ieee1275/linux.c |   2 +-
 grub-core/net/drivers/efi/efinet.c|  10 +-
 grub-core/net/net.c   |   2 +-
 include/grub/arm/linux.h  |   7 +-
 include/grub/arm64/linux.h|   5 +-
 include/grub/efi/api.h|  40 +
 include/grub/efi/efi.h|   4 +-
 include/grub/efi/pe32.h   |   5 +-
 include/grub/linux.h  |   2 +-
 include/grub/net.h|   3 +-
 23 files changed, 245 insertions(+), 65 deletions(-)

-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3 1/8] loader: drop argv[] argument in grub_initrd_load()

2022-08-18 Thread Ard Biesheuvel
From: Nikita Ermakov 

In the case of an error grub_initrd_load() uses argv[] to print the
filename that caused the error. It is also possible to obtain the
filename from the file handles and there is no need to duplicate that
information in argv[], so let's drop it.

Signed-off-by: Nikita Ermakov 
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Daniel Kiper 
---
 grub-core/loader/arm/linux.c  | 2 +-
 grub-core/loader/arm64/linux.c| 2 +-
 grub-core/loader/i386/linux.c | 2 +-
 grub-core/loader/i386/pc/linux.c  | 2 +-
 grub-core/loader/i386/xen.c   | 3 +--
 grub-core/loader/ia64/efi/linux.c | 2 +-
 grub-core/loader/linux.c  | 4 ++--
 grub-core/loader/mips/linux.c | 2 +-
 grub-core/loader/powerpc/ieee1275/linux.c | 2 +-
 grub-core/loader/sparc64/ieee1275/linux.c | 2 +-
 include/grub/linux.h  | 2 +-
 11 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/grub-core/loader/arm/linux.c b/grub-core/loader/arm/linux.c
index 30b366601c39..f00b538ebad0 100644
--- a/grub-core/loader/arm/linux.c
+++ b/grub-core/loader/arm/linux.c
@@ -422,7 +422,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
((unused)),
   grub_dprintf ("loader", "Loading initrd to 0x%08x\n",
(grub_addr_t) initrd_start);
 
-  if (grub_initrd_load (&initrd_ctx, argv, (void *) initrd_start))
+  if (grub_initrd_load (&initrd_ctx, (void *) initrd_start))
 goto fail;
 
   initrd_end = initrd_start + size;
diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index ef3e9f9444ca..aed7a200b848 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -266,7 +266,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
((unused)),
   goto fail;
 }
 
-  if (grub_initrd_load (&initrd_ctx, argv, initrd_mem))
+  if (grub_initrd_load (&initrd_ctx, initrd_mem))
 goto fail;
 
   initrd_start = (grub_addr_t) initrd_mem;
diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
index c5984d4b27e9..edd6c2bb1d23 100644
--- a/grub-core/loader/i386/linux.c
+++ b/grub-core/loader/i386/linux.c
@@ -1107,7 +1107,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
((unused)),
 initrd_mem_target = get_physical_target_address (ch);
   }
 
-  if (grub_initrd_load (&initrd_ctx, argv, initrd_mem))
+  if (grub_initrd_load (&initrd_ctx, initrd_mem))
 goto fail;
 
   grub_dprintf ("linux", "Initrd, addr=0x%x, size=0x%x\n",
diff --git a/grub-core/loader/i386/pc/linux.c b/grub-core/loader/i386/pc/linux.c
index 7b89d431051e..4adeee9ae001 100644
--- a/grub-core/loader/i386/pc/linux.c
+++ b/grub-core/loader/i386/pc/linux.c
@@ -462,7 +462,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
((unused)),
 initrd_addr = get_physical_target_address (ch);
   }
 
-  if (grub_initrd_load (&initrd_ctx, argv, initrd_chunk))
+  if (grub_initrd_load (&initrd_ctx, initrd_chunk))
 goto fail;
 
   lh->ramdisk_image = initrd_addr;
diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
index cd24874ca324..3b856e842709 100644
--- a/grub-core/loader/i386/xen.c
+++ b/grub-core/loader/i386/xen.c
@@ -809,8 +809,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
((unused)),
   if (err)
goto fail;
 
-  if (grub_initrd_load (&initrd_ctx, argv,
-   get_virtual_current_address (ch)))
+  if (grub_initrd_load (&initrd_ctx, get_virtual_current_address (ch)))
goto fail;
 }
 
diff --git a/grub-core/loader/ia64/efi/linux.c 
b/grub-core/loader/ia64/efi/linux.c
index 41266109e03c..fb9b961f71a2 100644
--- a/grub-core/loader/ia64/efi/linux.c
+++ b/grub-core/loader/ia64/efi/linux.c
@@ -563,7 +563,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
((unused)),
   grub_dprintf ("linux", "[addr=0x%lx, size=0x%lx]\n",
(grub_uint64_t) initrd_mem, initrd_size);
 
-  if (grub_initrd_load (&initrd_ctx, argv, initrd_mem))
+  if (grub_initrd_load (&initrd_ctx, initrd_mem))
 goto fail;
  fail:
   grub_initrd_close (&initrd_ctx);
diff --git a/grub-core/loader/linux.c b/grub-core/loader/linux.c
index ade9b2fef470..830360172c3d 100644
--- a/grub-core/loader/linux.c
+++ b/grub-core/loader/linux.c
@@ -271,7 +271,7 @@ grub_initrd_close (struct grub_linux_initrd_context 
*initrd_ctx)
 
 grub_err_t
 grub_initrd_load (struct grub_linux_initrd_context *initrd_ctx,
- char *argv[], void *target)
+ void *target)
 {
   grub_uint8_t *ptr = target;
   int i;
@@ -317,7 +317,7 @@ grub_initrd_load (struct grub_linux_initrd_context 
*initrd_ctx,
{
  if (!grub_errno)
grub_error (GRUB_ERR_FILE_READ_ERROR, N_("premature end of file 
%s"),
-   argv[i]);
+   initrd_ctx->components[i].file->name);
  grub_initrd_close (initrd_ctx);
  return grub_errno;
}
diff --git a/grub-core/loader/mips/linux.c b/grub

[PATCH v3 2/8] efi: move MS-DOS stub out of generic PE header definition

2022-08-18 Thread Ard Biesheuvel
The PE/COFF spec permits the COFF signature and file header to appear
anywhere in the file, and the actual offset is recorded in 4 byte
little endian field at offset 0x3c of the image.

When GRUB is emitted as a PE/COFF binary, we reuse the 128 byte MS-DOS
stub (even for non-x86 architectures), putting the COFF signature and
file header at offset 0x80. However, other PE/COFF images may use
different values, and non-x86 Linux kernels use an offset of 0x40
instead.

So let's get rid of the grub_pe32_header struct from pe32.h, given that
it does not represent anything defined by the PE/COFF spec. Instead,
use the GRUB_PE32_MSDOS_STUB_SIZE macro explicitly to reference the
COFF header in the only place in the code where we rely on this.

The remaining fields are moved into a struct grub_coff_image_header,
which we will use later to access COFF header fields of arbitrary
images (and which may therefore appear at different offsets)

Signed-off-by: Ard Biesheuvel 
---
 grub-core/kern/efi/efi.c | 5 +++--
 include/grub/efi/pe32.h  | 5 +
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index e8a976a22f15..8bef81663853 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -302,7 +302,7 @@ grub_addr_t
 grub_efi_modules_addr (void)
 {
   grub_efi_loaded_image_t *image;
-  struct grub_pe32_header *header;
+  struct grub_coff_image_header *header;
   struct grub_pe32_coff_header *coff_header;
   struct grub_pe32_section_table *sections;
   struct grub_pe32_section_table *section;
@@ -313,7 +313,8 @@ grub_efi_modules_addr (void)
   if (! image)
 return 0;
 
-  header = image->image_base;
+  header = (struct grub_coff_image_header *) ((char *) image->image_base
+ + GRUB_PE32_MSDOS_STUB_SIZE);
   coff_header = &(header->coff_header);
   sections
 = (struct grub_pe32_section_table *) ((char *) coff_header
diff --git a/include/grub/efi/pe32.h b/include/grub/efi/pe32.h
index 0ed8781f0376..a2da4b318c85 100644
--- a/include/grub/efi/pe32.h
+++ b/include/grub/efi/pe32.h
@@ -254,11 +254,8 @@ struct grub_pe32_section_table
 
 #define GRUB_PE32_SIGNATURE_SIZE 4
 
-struct grub_pe32_header
+struct grub_coff_image_header
 {
-  /* This should be filled in with GRUB_PE32_MSDOS_STUB.  */
-  grub_uint8_t msdos_stub[GRUB_PE32_MSDOS_STUB_SIZE];
-
   /* This is always PE\0\0.  */
   char signature[GRUB_PE32_SIGNATURE_SIZE];
 
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3 3/8] arm64/linux: Remove magic number header field check

2022-08-18 Thread Ard Biesheuvel
The 'ARM\x64' magic number in the file header identifies an image as one
that implements the bare metal boot protocol, allowing the loader to
simply move the file to a suitably aligned address in memory, with
sufficient headroom for the trailing .bss segment (the required memory
size is described in the header as well).

Note of this matters for GRUB, as it only supports EFI boot. EFI does
not care about this magic number, and nor should GRUB: this prevents us
from booting other PE linux images, such as the generic EFI zboot
decompressor, which is a pure PE/COFF image, and does not implement the
bare metal boot protocol.

So drop the magic number check.

Signed-off-by: Ard Biesheuvel 
---
 grub-core/loader/arm64/linux.c | 3 ---
 include/grub/arm/linux.h   | 1 -
 include/grub/arm64/linux.h | 1 -
 3 files changed, 5 deletions(-)

diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index aed7a200b848..b5b559c236e0 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -51,9 +51,6 @@ static grub_addr_t initrd_end;
 grub_err_t
 grub_arch_efi_linux_check_image (struct linux_arch_kernel_header * lh)
 {
-  if (lh->magic != GRUB_LINUX_ARMXX_MAGIC_SIGNATURE)
-return grub_error(GRUB_ERR_BAD_OS, "invalid magic number");
-
   if ((lh->code0 & 0x) != GRUB_PE32_MAGIC)
 return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
   N_("plain image kernel not supported - rebuild with 
CONFIG_(U)EFI_STUB enabled"));
diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h
index bcd5a7eb186e..bfab334dd87f 100644
--- a/include/grub/arm/linux.h
+++ b/include/grub/arm/linux.h
@@ -35,7 +35,6 @@ struct linux_arm_kernel_header {
 };
 
 #if defined(__arm__)
-# define GRUB_LINUX_ARMXX_MAGIC_SIGNATURE GRUB_LINUX_ARM_MAGIC_SIGNATURE
 # define linux_arch_kernel_header linux_arm_kernel_header
 #endif
 
diff --git a/include/grub/arm64/linux.h b/include/grub/arm64/linux.h
index 7e22b4ab6990..96f1494e05a2 100644
--- a/include/grub/arm64/linux.h
+++ b/include/grub/arm64/linux.h
@@ -39,7 +39,6 @@ struct linux_arm64_kernel_header
 };
 
 #if defined(__aarch64__)
-# define GRUB_LINUX_ARMXX_MAGIC_SIGNATURE GRUB_LINUX_ARM64_MAGIC_SIGNATURE
 # define linux_arch_kernel_header linux_arm64_kernel_header
 #endif
 
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3 6/8] efi/efinet: Don't close connections at fini_hw() time

2022-08-18 Thread Ard Biesheuvel
When GRUB runs on top of EFI firmware, it only has access to block and
network device abstractions exposed by the firmware, and it is up to the
firmware to quiesce the underlying hardware when handing over to the OS.

This is especially important for network devices, to prevent incoming
packets from being DMA'd straight into memory after the OS has taken
over but before it has managed to reconfigure the network hardware.

GRUB handles this by means of the grub_net_fini_hw() preboot hook, which
is executed before calling into the booted image. This means that all
network devices disappear or become inoperable before the EFI stub
executes on EFI targeted builds. This is problematic as it prevents the
EFI stub from calling back into GRUB provided protocols such as
LoadFile2 for the initrd, which we will provide in a subsequent patch.

So add a flag that indicates to the network core that EFI network
devices should not be closed when grub_net_fini_hw() is called.

Signed-off-by: Ard Biesheuvel 
---
 grub-core/net/drivers/efi/efinet.c | 10 +-
 grub-core/net/net.c|  2 +-
 include/grub/net.h |  3 ++-
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/grub-core/net/drivers/efi/efinet.c 
b/grub-core/net/drivers/efi/efinet.c
index 73343d26d9e1..5adf5f40f492 100644
--- a/grub-core/net/drivers/efi/efinet.c
+++ b/grub-core/net/drivers/efi/efinet.c
@@ -320,7 +320,15 @@ grub_efinet_findcards (void)
 
   card->name = grub_xasprintf ("efinet%d", i++);
   card->driver = &efidriver;
-  card->flags = 0;
+  /*
+   * EFI network devices are abstract SNP protocol instances, and the
+   * firmware is in charge of ensuring that they will be torn down when the
+   * OS loader hands off to the OS proper. Closing them as part of the
+   * preboot cleanup is therefore unnecessary, and undesirable, as it
+   * prevents us from using the network connection in a protocal callback
+   * such as LoadFile2 for initrd loading.
+   */
+  card->flags = GRUB_NET_CARD_NO_CLOSE_ON_FINI_HW;
   card->default_address.type = GRUB_NET_LINK_LEVEL_PROTOCOL_ETHERNET;
   grub_memcpy (card->default_address.mac,
   net->mode->current_address,
diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index 064e7114e012..7046dc57890a 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -1787,7 +1787,7 @@ grub_net_fini_hw (int noreturn __attribute__ ((unused)))
 {
   struct grub_net_card *card;
   FOR_NET_CARDS (card)
-if (card->opened)
+if (card->opened && !(card->flags & GRUB_NET_CARD_NO_CLOSE_ON_FINI_HW))
   {
if (card->driver->close)
  card->driver->close (card);
diff --git a/include/grub/net.h b/include/grub/net.h
index a64a04cc80b1..79cba357ae6a 100644
--- a/include/grub/net.h
+++ b/include/grub/net.h
@@ -64,7 +64,8 @@ typedef enum grub_net_interface_flags
 typedef enum grub_net_card_flags
   {
 GRUB_NET_CARD_HWADDRESS_IMMUTABLE = 1,
-GRUB_NET_CARD_NO_MANUAL_INTERFACES = 2
+GRUB_NET_CARD_NO_MANUAL_INTERFACES = 2,
+GRUB_NET_CARD_NO_CLOSE_ON_FINI_HW = 4
   } grub_net_card_flags_t;
 
 struct grub_net_card;
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3 4/8] linux/arm: unify ARM/arm64 vs Xen PE/COFF header handling

2022-08-18 Thread Ard Biesheuvel
Xen has its own version of the image header, to account for the
additional PE/COFF header fields. Since we are adding references to
those in the shared EFI loader code, update the common definitions
and drop the Xen specific one which no longer has a purpose.

Signed-off-by: Ard Biesheuvel 
---
 grub-core/loader/arm64/linux.c| 12 +-
 grub-core/loader/arm64/xen_boot.c | 23 
 include/grub/arm/linux.h  |  6 +
 include/grub/arm64/linux.h|  4 
 include/grub/efi/efi.h|  4 +++-
 5 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index b5b559c236e0..7c0f17cf933d 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -49,8 +49,13 @@ static grub_addr_t initrd_start;
 static grub_addr_t initrd_end;
 
 grub_err_t
-grub_arch_efi_linux_check_image (struct linux_arch_kernel_header * lh)
+grub_arch_efi_linux_load_image_header (grub_file_t file,
+  struct linux_arch_kernel_header * lh)
 {
+  grub_file_seek (file, 0);
+  if (grub_file_read (file, lh, sizeof (*lh)) < (long) sizeof (*lh))
+return grub_error(GRUB_ERR_FILE_READ_ERROR, "failed to read Linux image 
header");
+
   if ((lh->code0 & 0x) != GRUB_PE32_MAGIC)
 return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
   N_("plain image kernel not supported - rebuild with 
CONFIG_(U)EFI_STUB enabled"));
@@ -301,10 +306,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ 
((unused)),
 
   kernel_size = grub_file_size (file);
 
-  if (grub_file_read (file, &lh, sizeof (lh)) < (long) sizeof (lh))
-return grub_errno;
-
-  if (grub_arch_efi_linux_check_image (&lh) != GRUB_ERR_NONE)
+  if (grub_arch_efi_linux_load_image_header (file, &lh) != GRUB_ERR_NONE)
 goto fail;
 
   grub_loader_unset();
diff --git a/grub-core/loader/arm64/xen_boot.c 
b/grub-core/loader/arm64/xen_boot.c
index 22cc25eccd9d..e5895ee78218 100644
--- a/grub-core/loader/arm64/xen_boot.c
+++ b/grub-core/loader/arm64/xen_boot.c
@@ -31,7 +31,6 @@
 #include 
 #include 
 #include 
-#include  /* required by struct xen_hypervisor_header */
 #include 
 #include 
 
@@ -65,18 +64,6 @@ enum module_type
 };
 typedef enum module_type module_type_t;
 
-struct xen_hypervisor_header
-{
-  struct linux_arm64_kernel_header efi_head;
-
-  /* This is always PE\0\0.  */
-  grub_uint8_t signature[GRUB_PE32_SIGNATURE_SIZE];
-  /* The COFF file header.  */
-  struct grub_pe32_coff_header coff_header;
-  /* The Optional header.  */
-  struct grub_pe64_optional_header optional_header;
-};
-
 struct xen_boot_binary
 {
   struct xen_boot_binary *next;
@@ -452,7 +439,7 @@ static grub_err_t
 grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ ((unused)),
 int argc, char *argv[])
 {
-  struct xen_hypervisor_header sh;
+  struct linux_arm64_kernel_header lh;
   grub_file_t file = NULL;
 
   grub_dl_ref (my_mod);
@@ -467,10 +454,7 @@ grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ 
((unused)),
   if (!file)
 goto fail;
 
-  if (grub_file_read (file, &sh, sizeof (sh)) != (long) sizeof (sh))
-goto fail;
-  if (grub_arch_efi_linux_check_image
-  ((struct linux_arch_kernel_header *) &sh) != GRUB_ERR_NONE)
+  if (grub_arch_efi_linux_load_image_header (file, &lh) != GRUB_ERR_NONE)
 goto fail;
   grub_file_seek (file, 0);
 
@@ -484,7 +468,8 @@ grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ 
((unused)),
 return grub_errno;
 
   xen_hypervisor->is_hypervisor = 1;
-  xen_hypervisor->align = (grub_size_t) sh.optional_header.section_alignment;
+  xen_hypervisor->align
+= (grub_size_t) lh.coff_image_header.optional_header.section_alignment;
 
   xen_boot_binary_load (xen_hypervisor, file, argc, argv);
   if (grub_errno == GRUB_ERR_NONE)
diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h
index bfab334dd87f..f2a2c0379795 100644
--- a/include/grub/arm/linux.h
+++ b/include/grub/arm/linux.h
@@ -22,6 +22,8 @@
 
 #include "system.h"
 
+#include 
+
 #define GRUB_LINUX_ARM_MAGIC_SIGNATURE 0x016f2818
 
 struct linux_arm_kernel_header {
@@ -32,6 +34,10 @@ struct linux_arm_kernel_header {
   grub_uint32_t end;   /* _edata */
   grub_uint32_t reserved2[3];
   grub_uint32_t hdr_offset;
+
+#if defined GRUB_MACHINE_EFI
+  struct grub_coff_image_header coff_image_header;
+#endif
 };
 
 #if defined(__arm__)
diff --git a/include/grub/arm64/linux.h b/include/grub/arm64/linux.h
index 96f1494e05a2..e00bbcfa2ff2 100644
--- a/include/grub/arm64/linux.h
+++ b/include/grub/arm64/linux.h
@@ -21,6 +21,8 @@
 
 #include 
 
+#include 
+
 #define GRUB_LINUX_ARM64_MAGIC_SIGNATURE 0x644d5241 /* 'ARM\x64' */
 
 /* From linux/Documentation/arm64/booting.txt */
@@ -36,6 +38,8 @@ struct linux_arm64_kernel_header
   grub_uint64_t res4;  /* reserved */
   grub_uint32_t magic; /* Magic number, little endian, "ARM\x64" */
   grub_uint32_t hdr_offs

[PATCH v3 5/8] linux/arm: account for COFF headers appearing at unexpected offsets

2022-08-18 Thread Ard Biesheuvel
The way we load the Linux and PE/COFF image headers depends on a fixed
placement of the COFF header at offset 0x40 into the file. This is a
reasonable default, given that this is where Linux emits it today.
However, in order to comply with the PE/COFF spec, which allows this
header to appear anywhere in the file, let's ensure that we read the
header from where it actually appears in the file if it is not located
at offset 0x40.

Signed-off-by: Ard Biesheuvel 
---
 grub-core/loader/arm64/linux.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index 7c0f17cf933d..56ba8d0a6ea3 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -63,6 +63,21 @@ grub_arch_efi_linux_load_image_header (grub_file_t file,
   grub_dprintf ("linux", "UEFI stub kernel:\n");
   grub_dprintf ("linux", "PE/COFF header @ %08x\n", lh->hdr_offset);
 
+  /*
+   * The PE/COFF spec permits the COFF header to appear anywhere in the file, 
so
+   * we need to double check whether it was where we expected it, and if not, 
we
+   * must load it from the correct offset into the coff_image_header field of
+   * struct linux_arch_kernel_header.
+   */
+  if ((grub_uint8_t *) lh + lh->hdr_offset != (grub_uint8_t *) 
&lh->coff_image_header)
+{
+  grub_file_seek (file, lh->hdr_offset);
+
+  if (grub_file_read (file, &lh->coff_image_header, sizeof(struct 
grub_coff_image_header))
+ != sizeof(struct grub_coff_image_header))
+   return grub_error(GRUB_ERR_FILE_READ_ERROR, "failed to read COFF image 
header");
+}
+
   return GRUB_ERR_NONE;
 }
 
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3 7/8] efi: implement LoadFile2 initrd loading protocol for Linux

2022-08-18 Thread Ard Biesheuvel
Recent Linux kernels will invoke the LoadFile2 protocol installed on
a well-known vendor media path to load the initrd if it is exposed by
the firmware. Using this method is preferred for two reasons:
- the Linux kernel is in charge of allocating the memory, and so it can
  implement any placement policy it wants (given that these tend to
  change between kernel versions),
- it is no longer necessary to modify the device tree provided by the
  firmware.

So let's install this protocol when handling the 'initrd' command if
such a recent kernel was detected (based on the PE/COFF image version),
and defer loading the initrd contents until the point where the kernel
invokes the LoadFile2 protocol.

Signed-off-by: Ard Biesheuvel 
---
 grub-core/commands/efi/lsefi.c |   1 +
 grub-core/loader/arm64/linux.c | 123 +++-
 include/grub/efi/api.h |  40 +++
 3 files changed, 163 insertions(+), 1 deletion(-)

diff --git a/grub-core/commands/efi/lsefi.c b/grub-core/commands/efi/lsefi.c
index f46ba3b49384..c304d25ccdd6 100644
--- a/grub-core/commands/efi/lsefi.c
+++ b/grub-core/commands/efi/lsefi.c
@@ -55,6 +55,7 @@ struct known_protocol
 { GRUB_EFI_ABSOLUTE_POINTER_PROTOCOL_GUID, "absolute pointer" },
 { GRUB_EFI_DRIVER_BINDING_PROTOCOL_GUID, "EFI driver binding" },
 { GRUB_EFI_LOAD_FILE_PROTOCOL_GUID, "load file" },
+{ GRUB_EFI_LOAD_FILE2_PROTOCOL_GUID, "load file2" },
 { GRUB_EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID, "simple FS" },
 { GRUB_EFI_TAPE_IO_PROTOCOL_GUID, "tape I/O" },
 { GRUB_EFI_UNICODE_COLLATION_PROTOCOL_GUID, "unicode collation" },
diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index 56ba8d0a6ea3..8b7fbb35fb72 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -48,6 +48,39 @@ static grub_uint32_t cmdline_size;
 static grub_addr_t initrd_start;
 static grub_addr_t initrd_end;
 
+static struct grub_linux_initrd_context initrd_ctx = { 0, 0, 0 };
+static grub_efi_handle_t initrd_lf2_handle = NULL;
+static int initrd_use_loadfile2 = 0;
+
+static grub_efi_guid_t load_file2_guid = GRUB_EFI_LOAD_FILE2_PROTOCOL_GUID;
+static grub_efi_guid_t device_path_guid = GRUB_EFI_DEVICE_PATH_GUID;
+
+static initrd_media_device_path_t initrd_lf2_device_path = {
+  {
+{
+  GRUB_EFI_MEDIA_DEVICE_PATH_TYPE,
+  GRUB_EFI_VENDOR_MEDIA_DEVICE_PATH_SUBTYPE,
+  sizeof(grub_efi_vendor_media_device_path_t),
+},
+LINUX_EFI_INITRD_MEDIA_GUID
+  }, {
+GRUB_EFI_END_DEVICE_PATH_TYPE,
+GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE,
+sizeof(grub_efi_device_path_t)
+  }
+};
+
+static grub_efi_status_t __grub_efi_api
+grub_efi_initrd_load_file2(grub_efi_load_file2_t *this,
+   grub_efi_device_path_t *device_path,
+   grub_efi_boolean_t boot_policy,
+   grub_efi_uintn_t *buffer_size,
+   void *buffer);
+
+static grub_efi_load_file2_t initrd_lf2 = {
+  grub_efi_initrd_load_file2
+};
+
 grub_err_t
 grub_arch_efi_linux_load_image_header (grub_file_t file,
   struct linux_arch_kernel_header * lh)
@@ -78,6 +111,18 @@ grub_arch_efi_linux_load_image_header (grub_file_t file,
return grub_error(GRUB_ERR_FILE_READ_ERROR, "failed to read COFF image 
header");
 }
 
+  /*
+   * Linux kernels built for any architecture are guaranteed to support the
+   * LoadFile2 based initrd loading protocol if the image version is >= 1.
+   */
+  if (lh->coff_image_header.optional_header.major_image_version >= 1)
+initrd_use_loadfile2 = 1;
+  else
+initrd_use_loadfile2 = 0;
+
+  grub_dprintf ("linux", "LoadFile2 initrd loading %sabled\n",
+initrd_use_loadfile2 ? "en" : "dis");
+
   return GRUB_ERR_NONE;
 }
 
@@ -197,6 +242,8 @@ grub_linux_boot (void)
 static grub_err_t
 grub_linux_unload (void)
 {
+  grub_efi_boot_services_t *b = grub_efi_system_table->boot_services;
+
   grub_dl_unref (my_mod);
   loaded = 0;
   if (initrd_start)
@@ -208,6 +255,18 @@ grub_linux_unload (void)
 grub_efi_free_pages ((grub_addr_t) kernel_addr,
 GRUB_EFI_BYTES_TO_PAGES (kernel_size));
   grub_fdt_unload ();
+
+  if (initrd_lf2_handle)
+{
+  b->uninstall_multiple_protocol_interfaces (initrd_lf2_handle,
+ &load_file2_guid,
+ &initrd_lf2,
+ &device_path_guid,
+ &initrd_lf2_device_path,
+ NULL);
+  initrd_lf2_handle = NULL;
+  initrd_use_loadfile2 = 0;
+}
   return GRUB_ERR_NONE;
 }
 
@@ -247,13 +306,50 @@ allocate_initrd_mem (int initrd_pages)
   GRUB_EFI_LOADER_DATA);
 }
 
+static grub_efi_status_t __grub_efi_api
+grub_efi_initrd_load_file2(grub_efi_load_file2_t *this,
+  

[PATCH v3 8/8] linux: ignore FDT unless we need to modify it

2022-08-18 Thread Ard Biesheuvel
Now that we implemented support for the LoadFile2 protocol for initrd
loading, there is no longer a need to pass the initrd parameters via
the device tree. This means that when the LoadFile2 protocol is being
used, there is no reason to update the device tree in the first place,
and so we can ignore it entirely.

The only remaining reason to deal with the devicetree is if we are
using the 'devicetree' command to load one from disk, so tweak the
logic in grub_fdt_install() to take that into account.

Signed-off-by: Ard Biesheuvel 
Reviewed-by: Leif Lindholm 
---
 grub-core/loader/arm64/linux.c | 22 ++--
 grub-core/loader/efi/fdt.c |  7 +--
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index 8b7fbb35fb72..36396e9f7ff5 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -133,21 +133,21 @@ finalize_params_linux (void)
 
   void *fdt;
 
-  fdt = grub_fdt_load (GRUB_EFI_LINUX_FDT_EXTRA_SPACE);
+  /* Set initrd info */
+  if (initrd_start && initrd_end > initrd_start)
+{
+  fdt = grub_fdt_load (GRUB_EFI_LINUX_FDT_EXTRA_SPACE);
 
-  if (!fdt)
-goto failure;
+  if (!fdt)
+   goto failure;
 
-  node = grub_fdt_find_subnode (fdt, 0, "chosen");
-  if (node < 0)
-node = grub_fdt_add_subnode (fdt, 0, "chosen");
+  node = grub_fdt_find_subnode (fdt, 0, "chosen");
+  if (node < 0)
+   node = grub_fdt_add_subnode (fdt, 0, "chosen");
 
-  if (node < 1)
-goto failure;
+  if (node < 1)
+   goto failure;
 
-  /* Set initrd info */
-  if (initrd_start && initrd_end > initrd_start)
-{
   grub_dprintf ("linux", "Initrd @ %p-%p\n",
(void *) initrd_start, (void *) initrd_end);
 
diff --git a/grub-core/loader/efi/fdt.c b/grub-core/loader/efi/fdt.c
index c86f283d756b..771d455c7319 100644
--- a/grub-core/loader/efi/fdt.c
+++ b/grub-core/loader/efi/fdt.c
@@ -89,13 +89,16 @@ grub_fdt_install (void)
   grub_efi_guid_t fdt_guid = GRUB_EFI_DEVICE_TREE_GUID;
   grub_efi_status_t status;
 
+  if (!fdt && !loaded_fdt)
+return GRUB_ERR_NONE;
+
   b = grub_efi_system_table->boot_services;
-  status = b->install_configuration_table (&fdt_guid, fdt);
+  status = b->install_configuration_table (&fdt_guid, fdt ?: loaded_fdt);
   if (status != GRUB_EFI_SUCCESS)
 return grub_error (GRUB_ERR_IO, "failed to install FDT");
 
   grub_dprintf ("fdt", "Installed/updated FDT configuration table @ %p\n",
-   fdt);
+   fdt ?: loaded_fdt);
   return GRUB_ERR_NONE;
 }
 
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3 0/5] Improve logic to check for fwsetup support

2022-08-18 Thread Robbie Harwood
In this version: add and check `fwsetup --is-supported, and include the
changes for Paul's review that were left out accidentally in the previous
version.

Be well,
--Robbie

Javier Martinez Canillas (2):
  templates: Check for EFI at runtime instead of config generation time
  efi: Print an error if boot to firmware setup is not supported

Robbie Harwood (3):
  commands/efi/efifwsetup: Add missing grub_free()s
  Make all grub_efi_guid_t variables static
  Don't display a uefi-firmware entry if it's not supported

 grub-core/commands/efi/efifwsetup.c  | 30 +++-
 grub-core/efiemu/i386/pc/cfgtables.c |  6 +++---
 grub-core/kern/efi/fdt.c |  2 +-
 grub-core/loader/efi/fdt.c   |  2 +-
 grub-core/term/efi/console.c |  2 +-
 util/grub.d/30_uefi-firmware.in  | 21 ---
 6 files changed, 35 insertions(+), 28 deletions(-)

-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3 3/5] templates: Check for EFI at runtime instead of config generation time

2022-08-18 Thread Robbie Harwood
From: Javier Martinez Canillas 

The 30_uefi-firmware template checks if an OsIndicationsSupported UEFI var
exists and EFI_OS_INDICATIONS_BOOT_TO_FW_UI bit is set, to decide whether
a "fwsetup" menu entry would be added or not to the GRUB menu.

But this has the problem that it will only work if the configuration file
was created on an UEFI machine that supports booting to a firmware UI.

This for example doesn't support creating GRUB config files when executing
on systems that support both UEFI and legacy BIOS booting. Since creating
the config file from legacy BIOS wouldn't allow to access the firmware UI.

To prevent this, make the template to unconditionally create the grub.cfg
snippet but check at runtime if was booted through UEFI to decide if this
entry should be added. That way it won't be added when booting with BIOS.

There's no need to check if EFI_OS_INDICATIONS_BOOT_TO_FW_UI bit is set,
since that's already done by the "fwsetup" command when is executed.

Signed-off-by: Javier Martinez Canillas 
Signed-off-by: Robbie Harwood 
---
 util/grub.d/30_uefi-firmware.in | 21 -
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/util/grub.d/30_uefi-firmware.in b/util/grub.d/30_uefi-firmware.in
index d344d3883d..b6041b55e2 100644
--- a/util/grub.d/30_uefi-firmware.in
+++ b/util/grub.d/30_uefi-firmware.in
@@ -26,19 +26,14 @@ export TEXTDOMAINDIR="@localedir@"
 
 . "$pkgdatadir/grub-mkconfig_lib"
 
-EFI_VARS_DIR=/sys/firmware/efi/efivars
-EFI_GLOBAL_VARIABLE=8be4df61-93ca-11d2-aa0d-00e098032b8c
-OS_INDICATIONS="$EFI_VARS_DIR/OsIndicationsSupported-$EFI_GLOBAL_VARIABLE"
+LABEL="UEFI Firmware Settings"
 
-if [ -e "$OS_INDICATIONS" ] && \
-   [ "$(( $(printf 0x%x \'"$(cat $OS_INDICATIONS | cut -b5)"\') & 1 ))" = 1 ]; 
then
-  LABEL="UEFI Firmware Settings"
+gettext_printf "Adding boot menu entry for UEFI Firmware Settings ...\n" >&2
 
-  gettext_printf "Adding boot menu entry for UEFI Firmware Settings ...\n" >&2
-
-  cat << EOF
-menuentry '$LABEL' \$menuentry_id_option 'uefi-firmware' {
-   fwsetup
-}
-EOF
+cat << EOF
+if [ "\$grub_platform" = "efi" ]; then
+   menuentry '$LABEL' \$menuentry_id_option 'uefi-firmware' {
+   fwsetup
+   }
 fi
+EOF
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3 2/5] Make all grub_efi_guid_t variables static

2022-08-18 Thread Robbie Harwood
This is believed to result in smaller code.

Signed-off-by: Robbie Harwood 
---
 grub-core/commands/efi/efifwsetup.c  | 4 ++--
 grub-core/efiemu/i386/pc/cfgtables.c | 6 +++---
 grub-core/kern/efi/fdt.c | 2 +-
 grub-core/loader/efi/fdt.c   | 2 +-
 grub-core/term/efi/console.c | 2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/grub-core/commands/efi/efifwsetup.c 
b/grub-core/commands/efi/efifwsetup.c
index b5b1f106c6..51bb2ace3b 100644
--- a/grub-core/commands/efi/efifwsetup.c
+++ b/grub-core/commands/efi/efifwsetup.c
@@ -36,7 +36,7 @@ grub_cmd_fwsetup (grub_command_t cmd __attribute__ ((unused)),
   grub_efi_uint64_t os_indications = GRUB_EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
   grub_err_t status;
   grub_size_t oi_size;
-  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
+  static grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
 
   grub_efi_get_variable ("OsIndications", &global, &oi_size,
 (void **) &old_os_indications);
@@ -63,7 +63,7 @@ efifwsetup_is_supported (void)
 {
   grub_efi_uint64_t *os_indications_supported = NULL;
   grub_size_t oi_size = 0;
-  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
+  static grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
   grub_boolean_t ret = 0;
 
   grub_efi_get_variable ("OsIndicationsSupported", &global, &oi_size,
diff --git a/grub-core/efiemu/i386/pc/cfgtables.c 
b/grub-core/efiemu/i386/pc/cfgtables.c
index e5fffb7d4a..1098f0b79f 100644
--- a/grub-core/efiemu/i386/pc/cfgtables.c
+++ b/grub-core/efiemu/i386/pc/cfgtables.c
@@ -29,9 +29,9 @@ grub_machine_efiemu_init_tables (void)
 {
   void *table;
   grub_err_t err;
-  grub_efi_guid_t smbios = GRUB_EFI_SMBIOS_TABLE_GUID;
-  grub_efi_guid_t acpi20 = GRUB_EFI_ACPI_20_TABLE_GUID;
-  grub_efi_guid_t acpi = GRUB_EFI_ACPI_TABLE_GUID;
+  static grub_efi_guid_t smbios = GRUB_EFI_SMBIOS_TABLE_GUID;
+  static grub_efi_guid_t acpi20 = GRUB_EFI_ACPI_20_TABLE_GUID;
+  static grub_efi_guid_t acpi = GRUB_EFI_ACPI_TABLE_GUID;
 
   err = grub_efiemu_unregister_configuration_table (smbios);
   if (err)
diff --git a/grub-core/kern/efi/fdt.c b/grub-core/kern/efi/fdt.c
index 30100c61c1..24f955289f 100644
--- a/grub-core/kern/efi/fdt.c
+++ b/grub-core/kern/efi/fdt.c
@@ -24,7 +24,7 @@ void *
 grub_efi_get_firmware_fdt (void)
 {
   grub_efi_configuration_table_t *tables;
-  grub_efi_guid_t fdt_guid = GRUB_EFI_DEVICE_TREE_GUID;
+  static grub_efi_guid_t fdt_guid = GRUB_EFI_DEVICE_TREE_GUID;
   void *firmware_fdt = NULL;
   unsigned int i;
 
diff --git a/grub-core/loader/efi/fdt.c b/grub-core/loader/efi/fdt.c
index c86f283d75..80d7088747 100644
--- a/grub-core/loader/efi/fdt.c
+++ b/grub-core/loader/efi/fdt.c
@@ -86,7 +86,7 @@ grub_err_t
 grub_fdt_install (void)
 {
   grub_efi_boot_services_t *b;
-  grub_efi_guid_t fdt_guid = GRUB_EFI_DEVICE_TREE_GUID;
+  static grub_efi_guid_t fdt_guid = GRUB_EFI_DEVICE_TREE_GUID;
   grub_efi_status_t status;
 
   b = grub_efi_system_table->boot_services;
diff --git a/grub-core/term/efi/console.c b/grub-core/term/efi/console.c
index a3622e4fe5..532948a8e1 100644
--- a/grub-core/term/efi/console.c
+++ b/grub-core/term/efi/console.c
@@ -353,7 +353,7 @@ grub_console_getkeystatus (struct grub_term_input *term)
 static grub_err_t
 grub_efi_console_input_init (struct grub_term_input *term)
 {
-  grub_efi_guid_t text_input_ex_guid =
+  static grub_efi_guid_t text_input_ex_guid =
 GRUB_EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL_GUID;
 
   if (grub_efi_is_finished)
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3 4/5] efi: Print an error if boot to firmware setup is not supported

2022-08-18 Thread Robbie Harwood
From: Javier Martinez Canillas 

The "fwsetup" command is only registered if the firmware supports booting
to the firmware setup UI. But it could be possible that the GRUB config
already contains a "fwsetup" entry, because it was generated in a machine
that has support for this feature.

To prevent users getting an error like:

error: ../../grub-core/script/function.c:109:can't find command `fwsetup'.

if it is not supported by the firmware, let's just always register the
command but print a more accurate message if the firmware doesn't
support this option.

Signed-off-by: Javier Martinez Canillas 
Signed-off-by: Robbie Harwood 
---
 grub-core/commands/efi/efifwsetup.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/grub-core/commands/efi/efifwsetup.c 
b/grub-core/commands/efi/efifwsetup.c
index 51bb2ace3b..cb4b6ff18c 100644
--- a/grub-core/commands/efi/efifwsetup.c
+++ b/grub-core/commands/efi/efifwsetup.c
@@ -27,6 +27,8 @@
 
 GRUB_MOD_LICENSE ("GPLv3+");
 
+static grub_efi_boolean_t efifwsetup_is_supported (void);
+
 static grub_err_t
 grub_cmd_fwsetup (grub_command_t cmd __attribute__ ((unused)),
  int argc __attribute__ ((unused)),
@@ -38,6 +40,10 @@ grub_cmd_fwsetup (grub_command_t cmd __attribute__ 
((unused)),
   grub_size_t oi_size;
   static grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
 
+  if (!efifwsetup_is_supported ())
+ return grub_error (GRUB_ERR_INVALID_COMMAND,
+N_("reboot to firmware setup is not supported by 
the current firmware"));
+
   grub_efi_get_variable ("OsIndications", &global, &oi_size,
 (void **) &old_os_indications);
 
@@ -82,10 +88,8 @@ efifwsetup_is_supported (void)
 
 GRUB_MOD_INIT (efifwsetup)
 {
-  if (efifwsetup_is_supported ())
-cmd = grub_register_command ("fwsetup", grub_cmd_fwsetup, NULL,
-N_("Reboot into firmware setup menu."));
-
+  cmd = grub_register_command ("fwsetup", grub_cmd_fwsetup, NULL,
+   N_("Reboot into firmware setup menu."));
 }
 
 GRUB_MOD_FINI (efifwsetup)
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3 1/5] commands/efi/efifwsetup: Add missing grub_free()s

2022-08-18 Thread Robbie Harwood
Each call of grub_efi_get_variable() needs a grub_free().

Signed-off-by: Robbie Harwood 
---
 grub-core/commands/efi/efifwsetup.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/grub-core/commands/efi/efifwsetup.c 
b/grub-core/commands/efi/efifwsetup.c
index eaca032838..b5b1f106c6 100644
--- a/grub-core/commands/efi/efifwsetup.c
+++ b/grub-core/commands/efi/efifwsetup.c
@@ -44,6 +44,8 @@ grub_cmd_fwsetup (grub_command_t cmd __attribute__ ((unused)),
   if (old_os_indications != NULL && oi_size == sizeof (os_indications))
 os_indications |= *old_os_indications;
 
+  grub_free (old_os_indications);
+
   status = grub_efi_set_variable ("OsIndications", &global, &os_indications,
  sizeof (os_indications));
   if (status != GRUB_ERR_NONE)
@@ -62,17 +64,20 @@ efifwsetup_is_supported (void)
   grub_efi_uint64_t *os_indications_supported = NULL;
   grub_size_t oi_size = 0;
   grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
+  grub_boolean_t ret = 0;
 
   grub_efi_get_variable ("OsIndicationsSupported", &global, &oi_size,
 (void **) &os_indications_supported);
 
   if (!os_indications_supported)
-return 0;
+goto done;
 
   if (*os_indications_supported & GRUB_EFI_OS_INDICATIONS_BOOT_TO_FW_UI)
-return 1;
+ret = 1;
 
-  return 0;
+ done:
+  grub_free (os_indications_supported);
+  return ret;
 }
 
 GRUB_MOD_INIT (efifwsetup)
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3 5/5] Don't display a uefi-firmware entry if it's not supported

2022-08-18 Thread Robbie Harwood
Add a new --is-supported option to commands/efi/efifwsetup and
conditionalize display on it.

Signed-off-by: Robbie Harwood 
---
 grub-core/commands/efi/efifwsetup.c | 3 +++
 util/grub.d/30_uefi-firmware.in | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/grub-core/commands/efi/efifwsetup.c 
b/grub-core/commands/efi/efifwsetup.c
index cb4b6ff18c..52e47102ba 100644
--- a/grub-core/commands/efi/efifwsetup.c
+++ b/grub-core/commands/efi/efifwsetup.c
@@ -40,6 +40,9 @@ grub_cmd_fwsetup (grub_command_t cmd __attribute__ ((unused)),
   grub_size_t oi_size;
   static grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
 
+  if (argc >= 1 && strcmp(args[1], "--is-supported"))
+return !efifwsetup_is_supported ();
+
   if (!efifwsetup_is_supported ())
  return grub_error (GRUB_ERR_INVALID_COMMAND,
 N_("reboot to firmware setup is not supported by 
the current firmware"));
diff --git a/util/grub.d/30_uefi-firmware.in b/util/grub.d/30_uefi-firmware.in
index b6041b55e2..78aef67d78 100644
--- a/util/grub.d/30_uefi-firmware.in
+++ b/util/grub.d/30_uefi-firmware.in
@@ -31,7 +31,7 @@ LABEL="UEFI Firmware Settings"
 gettext_printf "Adding boot menu entry for UEFI Firmware Settings ...\n" >&2
 
 cat << EOF
-if [ "\$grub_platform" = "efi" ]; then
+if [ "\$grub_platform" = "efi" ] && fwsetup --is-supported; then
menuentry '$LABEL' \$menuentry_id_option 'uefi-firmware' {
fwsetup
}
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v4 1/5] commands/efi/efifwsetup: Add missing grub_free()s

2022-08-18 Thread Robbie Harwood
Each call of grub_efi_get_variable() needs a grub_free().

Signed-off-by: Robbie Harwood 
---
 grub-core/commands/efi/efifwsetup.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/grub-core/commands/efi/efifwsetup.c 
b/grub-core/commands/efi/efifwsetup.c
index eaca032838..b5b1f106c6 100644
--- a/grub-core/commands/efi/efifwsetup.c
+++ b/grub-core/commands/efi/efifwsetup.c
@@ -44,6 +44,8 @@ grub_cmd_fwsetup (grub_command_t cmd __attribute__ ((unused)),
   if (old_os_indications != NULL && oi_size == sizeof (os_indications))
 os_indications |= *old_os_indications;
 
+  grub_free (old_os_indications);
+
   status = grub_efi_set_variable ("OsIndications", &global, &os_indications,
  sizeof (os_indications));
   if (status != GRUB_ERR_NONE)
@@ -62,17 +64,20 @@ efifwsetup_is_supported (void)
   grub_efi_uint64_t *os_indications_supported = NULL;
   grub_size_t oi_size = 0;
   grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
+  grub_boolean_t ret = 0;
 
   grub_efi_get_variable ("OsIndicationsSupported", &global, &oi_size,
 (void **) &os_indications_supported);
 
   if (!os_indications_supported)
-return 0;
+goto done;
 
   if (*os_indications_supported & GRUB_EFI_OS_INDICATIONS_BOOT_TO_FW_UI)
-return 1;
+ret = 1;
 
-  return 0;
+ done:
+  grub_free (os_indications_supported);
+  return ret;
 }
 
 GRUB_MOD_INIT (efifwsetup)
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v4 2/5] Make all grub_efi_guid_t variables static

2022-08-18 Thread Robbie Harwood
This is believed to result in smaller code.

Signed-off-by: Robbie Harwood 
---
 grub-core/commands/efi/efifwsetup.c  | 4 ++--
 grub-core/efiemu/i386/pc/cfgtables.c | 6 +++---
 grub-core/kern/efi/fdt.c | 2 +-
 grub-core/loader/efi/fdt.c   | 2 +-
 grub-core/term/efi/console.c | 2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/grub-core/commands/efi/efifwsetup.c 
b/grub-core/commands/efi/efifwsetup.c
index b5b1f106c6..51bb2ace3b 100644
--- a/grub-core/commands/efi/efifwsetup.c
+++ b/grub-core/commands/efi/efifwsetup.c
@@ -36,7 +36,7 @@ grub_cmd_fwsetup (grub_command_t cmd __attribute__ ((unused)),
   grub_efi_uint64_t os_indications = GRUB_EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
   grub_err_t status;
   grub_size_t oi_size;
-  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
+  static grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
 
   grub_efi_get_variable ("OsIndications", &global, &oi_size,
 (void **) &old_os_indications);
@@ -63,7 +63,7 @@ efifwsetup_is_supported (void)
 {
   grub_efi_uint64_t *os_indications_supported = NULL;
   grub_size_t oi_size = 0;
-  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
+  static grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
   grub_boolean_t ret = 0;
 
   grub_efi_get_variable ("OsIndicationsSupported", &global, &oi_size,
diff --git a/grub-core/efiemu/i386/pc/cfgtables.c 
b/grub-core/efiemu/i386/pc/cfgtables.c
index e5fffb7d4a..1098f0b79f 100644
--- a/grub-core/efiemu/i386/pc/cfgtables.c
+++ b/grub-core/efiemu/i386/pc/cfgtables.c
@@ -29,9 +29,9 @@ grub_machine_efiemu_init_tables (void)
 {
   void *table;
   grub_err_t err;
-  grub_efi_guid_t smbios = GRUB_EFI_SMBIOS_TABLE_GUID;
-  grub_efi_guid_t acpi20 = GRUB_EFI_ACPI_20_TABLE_GUID;
-  grub_efi_guid_t acpi = GRUB_EFI_ACPI_TABLE_GUID;
+  static grub_efi_guid_t smbios = GRUB_EFI_SMBIOS_TABLE_GUID;
+  static grub_efi_guid_t acpi20 = GRUB_EFI_ACPI_20_TABLE_GUID;
+  static grub_efi_guid_t acpi = GRUB_EFI_ACPI_TABLE_GUID;
 
   err = grub_efiemu_unregister_configuration_table (smbios);
   if (err)
diff --git a/grub-core/kern/efi/fdt.c b/grub-core/kern/efi/fdt.c
index 30100c61c1..24f955289f 100644
--- a/grub-core/kern/efi/fdt.c
+++ b/grub-core/kern/efi/fdt.c
@@ -24,7 +24,7 @@ void *
 grub_efi_get_firmware_fdt (void)
 {
   grub_efi_configuration_table_t *tables;
-  grub_efi_guid_t fdt_guid = GRUB_EFI_DEVICE_TREE_GUID;
+  static grub_efi_guid_t fdt_guid = GRUB_EFI_DEVICE_TREE_GUID;
   void *firmware_fdt = NULL;
   unsigned int i;
 
diff --git a/grub-core/loader/efi/fdt.c b/grub-core/loader/efi/fdt.c
index c86f283d75..80d7088747 100644
--- a/grub-core/loader/efi/fdt.c
+++ b/grub-core/loader/efi/fdt.c
@@ -86,7 +86,7 @@ grub_err_t
 grub_fdt_install (void)
 {
   grub_efi_boot_services_t *b;
-  grub_efi_guid_t fdt_guid = GRUB_EFI_DEVICE_TREE_GUID;
+  static grub_efi_guid_t fdt_guid = GRUB_EFI_DEVICE_TREE_GUID;
   grub_efi_status_t status;
 
   b = grub_efi_system_table->boot_services;
diff --git a/grub-core/term/efi/console.c b/grub-core/term/efi/console.c
index a3622e4fe5..532948a8e1 100644
--- a/grub-core/term/efi/console.c
+++ b/grub-core/term/efi/console.c
@@ -353,7 +353,7 @@ grub_console_getkeystatus (struct grub_term_input *term)
 static grub_err_t
 grub_efi_console_input_init (struct grub_term_input *term)
 {
-  grub_efi_guid_t text_input_ex_guid =
+  static grub_efi_guid_t text_input_ex_guid =
 GRUB_EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL_GUID;
 
   if (grub_efi_is_finished)
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v4 0/5] Improve logic to check for fwsetup support

2022-08-18 Thread Robbie Harwood
In this version: fix wrong arg parsing in patch to address Glenn's review.

Be well,
--Robbie

Javier Martinez Canillas (2):
  templates: Check for EFI at runtime instead of config generation time
  efi: Print an error if boot to firmware setup is not supported

Robbie Harwood (3):
  commands/efi/efifwsetup: Add missing grub_free()s
  Make all grub_efi_guid_t variables static
  Don't display a uefi-firmware entry if it's not supported

 grub-core/commands/efi/efifwsetup.c  | 30 +++-
 grub-core/efiemu/i386/pc/cfgtables.c |  6 +++---
 grub-core/kern/efi/fdt.c |  2 +-
 grub-core/loader/efi/fdt.c   |  2 +-
 grub-core/term/efi/console.c |  2 +-
 util/grub.d/30_uefi-firmware.in  | 21 ---
 6 files changed, 35 insertions(+), 28 deletions(-)

-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v4 3/5] templates: Check for EFI at runtime instead of config generation time

2022-08-18 Thread Robbie Harwood
From: Javier Martinez Canillas 

The 30_uefi-firmware template checks if an OsIndicationsSupported UEFI var
exists and EFI_OS_INDICATIONS_BOOT_TO_FW_UI bit is set, to decide whether
a "fwsetup" menu entry would be added or not to the GRUB menu.

But this has the problem that it will only work if the configuration file
was created on an UEFI machine that supports booting to a firmware UI.

This for example doesn't support creating GRUB config files when executing
on systems that support both UEFI and legacy BIOS booting. Since creating
the config file from legacy BIOS wouldn't allow to access the firmware UI.

To prevent this, make the template to unconditionally create the grub.cfg
snippet but check at runtime if was booted through UEFI to decide if this
entry should be added. That way it won't be added when booting with BIOS.

There's no need to check if EFI_OS_INDICATIONS_BOOT_TO_FW_UI bit is set,
since that's already done by the "fwsetup" command when is executed.

Signed-off-by: Javier Martinez Canillas 
Signed-off-by: Robbie Harwood 
---
 util/grub.d/30_uefi-firmware.in | 21 -
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/util/grub.d/30_uefi-firmware.in b/util/grub.d/30_uefi-firmware.in
index d344d3883d..b6041b55e2 100644
--- a/util/grub.d/30_uefi-firmware.in
+++ b/util/grub.d/30_uefi-firmware.in
@@ -26,19 +26,14 @@ export TEXTDOMAINDIR="@localedir@"
 
 . "$pkgdatadir/grub-mkconfig_lib"
 
-EFI_VARS_DIR=/sys/firmware/efi/efivars
-EFI_GLOBAL_VARIABLE=8be4df61-93ca-11d2-aa0d-00e098032b8c
-OS_INDICATIONS="$EFI_VARS_DIR/OsIndicationsSupported-$EFI_GLOBAL_VARIABLE"
+LABEL="UEFI Firmware Settings"
 
-if [ -e "$OS_INDICATIONS" ] && \
-   [ "$(( $(printf 0x%x \'"$(cat $OS_INDICATIONS | cut -b5)"\') & 1 ))" = 1 ]; 
then
-  LABEL="UEFI Firmware Settings"
+gettext_printf "Adding boot menu entry for UEFI Firmware Settings ...\n" >&2
 
-  gettext_printf "Adding boot menu entry for UEFI Firmware Settings ...\n" >&2
-
-  cat << EOF
-menuentry '$LABEL' \$menuentry_id_option 'uefi-firmware' {
-   fwsetup
-}
-EOF
+cat << EOF
+if [ "\$grub_platform" = "efi" ]; then
+   menuentry '$LABEL' \$menuentry_id_option 'uefi-firmware' {
+   fwsetup
+   }
 fi
+EOF
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v4 5/5] Don't display a uefi-firmware entry if it's not supported

2022-08-18 Thread Robbie Harwood
Add a new --is-supported option to commands/efi/efifwsetup and
conditionalize display on it.

Signed-off-by: Robbie Harwood 
---
 grub-core/commands/efi/efifwsetup.c | 3 +++
 util/grub.d/30_uefi-firmware.in | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/grub-core/commands/efi/efifwsetup.c 
b/grub-core/commands/efi/efifwsetup.c
index cb4b6ff18c..53d23deea7 100644
--- a/grub-core/commands/efi/efifwsetup.c
+++ b/grub-core/commands/efi/efifwsetup.c
@@ -40,6 +40,9 @@ grub_cmd_fwsetup (grub_command_t cmd __attribute__ ((unused)),
   grub_size_t oi_size;
   static grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
 
+  if (argc >= 1 && grub_strcmp(args[0], "--is-supported") == 0)
+return !efifwsetup_is_supported ();
+
   if (!efifwsetup_is_supported ())
  return grub_error (GRUB_ERR_INVALID_COMMAND,
 N_("reboot to firmware setup is not supported by 
the current firmware"));
diff --git a/util/grub.d/30_uefi-firmware.in b/util/grub.d/30_uefi-firmware.in
index b6041b55e2..78aef67d78 100644
--- a/util/grub.d/30_uefi-firmware.in
+++ b/util/grub.d/30_uefi-firmware.in
@@ -31,7 +31,7 @@ LABEL="UEFI Firmware Settings"
 gettext_printf "Adding boot menu entry for UEFI Firmware Settings ...\n" >&2
 
 cat << EOF
-if [ "\$grub_platform" = "efi" ]; then
+if [ "\$grub_platform" = "efi" ] && fwsetup --is-supported; then
menuentry '$LABEL' \$menuentry_id_option 'uefi-firmware' {
fwsetup
}
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v4 4/5] efi: Print an error if boot to firmware setup is not supported

2022-08-18 Thread Robbie Harwood
From: Javier Martinez Canillas 

The "fwsetup" command is only registered if the firmware supports booting
to the firmware setup UI. But it could be possible that the GRUB config
already contains a "fwsetup" entry, because it was generated in a machine
that has support for this feature.

To prevent users getting an error like:

error: ../../grub-core/script/function.c:109:can't find command `fwsetup'.

if it is not supported by the firmware, let's just always register the
command but print a more accurate message if the firmware doesn't
support this option.

Signed-off-by: Javier Martinez Canillas 
Signed-off-by: Robbie Harwood 
---
 grub-core/commands/efi/efifwsetup.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/grub-core/commands/efi/efifwsetup.c 
b/grub-core/commands/efi/efifwsetup.c
index 51bb2ace3b..cb4b6ff18c 100644
--- a/grub-core/commands/efi/efifwsetup.c
+++ b/grub-core/commands/efi/efifwsetup.c
@@ -27,6 +27,8 @@
 
 GRUB_MOD_LICENSE ("GPLv3+");
 
+static grub_efi_boolean_t efifwsetup_is_supported (void);
+
 static grub_err_t
 grub_cmd_fwsetup (grub_command_t cmd __attribute__ ((unused)),
  int argc __attribute__ ((unused)),
@@ -38,6 +40,10 @@ grub_cmd_fwsetup (grub_command_t cmd __attribute__ 
((unused)),
   grub_size_t oi_size;
   static grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
 
+  if (!efifwsetup_is_supported ())
+ return grub_error (GRUB_ERR_INVALID_COMMAND,
+N_("reboot to firmware setup is not supported by 
the current firmware"));
+
   grub_efi_get_variable ("OsIndications", &global, &oi_size,
 (void **) &old_os_indications);
 
@@ -82,10 +88,8 @@ efifwsetup_is_supported (void)
 
 GRUB_MOD_INIT (efifwsetup)
 {
-  if (efifwsetup_is_supported ())
-cmd = grub_register_command ("fwsetup", grub_cmd_fwsetup, NULL,
-N_("Reboot into firmware setup menu."));
-
+  cmd = grub_register_command ("fwsetup", grub_cmd_fwsetup, NULL,
+   N_("Reboot into firmware setup menu."));
 }
 
 GRUB_MOD_FINI (efifwsetup)
-- 
2.35.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel