diff mbox series

[3/7] multiboot2: Add support for the load type header tag

Message ID 20240313150748.791236-4-ross.lagerwall@citrix.com (mailing list archive)
State New, archived
Headers show
Series GRUB: Supporting Secure Boot of xen.gz | expand

Commit Message

Ross Lagerwall March 13, 2024, 3:07 p.m. UTC 
The binary may expose its type using the load type header tag. Implement
it according to the specification.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 grub-core/loader/multiboot_mbi2.c | 45 ++++++++++++++++++++++++++++---
 include/grub/multiboot2.h         |  1 +
 include/multiboot2.h              | 13 +++++++++
 3 files changed, 56 insertions(+), 3 deletions(-)

Comments

Vladimir 'phcoder' Serbinenko March 15, 2024, 7:30 a.m. UTC | #1 
Not a full review. Just one blocking problem


>
>      }
> +  case MULTIBOOT_LOAD_TYPE_PE:
> +      grub_fatal ("Unsupported load type: %u\n", mld.load_type);
> +  default:
> +    /* should be impossible */
> +    grub_fatal ("Unknown load type: %u\n", mld.load_type);
>
Don't use grub_fatal for this. grub_fatal is only when continue to execute
grub is unwise. Here you just have an unsupported file. This is definitely
a GRUB_ERR_BAD_OS


>
Ross Lagerwall March 28, 2024, 2:58 p.m. UTC | #2 
On Fri, Mar 15, 2024 at 7:31 AM Vladimir 'phcoder' Serbinenko
<phcoder@gmail.com> wrote:
>
> Not a full review. Just one blocking problem
>
>>
>>
>>      }
>> +  case MULTIBOOT_LOAD_TYPE_PE:
>> +      grub_fatal ("Unsupported load type: %u\n", mld.load_type);
>> +  default:
>> +    /* should be impossible */
>> +    grub_fatal ("Unknown load type: %u\n", mld.load_type);
>
> Don't use grub_fatal for this. grub_fatal is only when continue to execute grub is unwise. Here you just have an unsupported file. This is definitely a GRUB_ERR_BAD_OS
>

Noted... I'll fix that.

Thanks,
Ross
diff mbox series

Patch

diff --git a/grub-core/loader/multiboot_mbi2.c b/grub-core/loader/multiboot_mbi2.c
index 00a48413c013..6ef4f265070a 100644
--- a/grub-core/loader/multiboot_mbi2.c
+++ b/grub-core/loader/multiboot_mbi2.c
@@ -119,10 +119,12 @@  grub_multiboot2_load (grub_file_t file, const char *filename)
   grub_uint32_t console_required = 0;
   struct multiboot_header_tag_framebuffer *fbtag = NULL;
   int accepted_consoles = GRUB_MULTIBOOT2_CONSOLE_EGA_TEXT;
+  struct multiboot_header_tag_load_type *load_type_tag;
   mbi_load_data_t mld;
 
   mld.mbi_ver = 2;
   mld.relocatable = 0;
+  mld.load_type = MULTIBOOT_LOAD_TYPE_TOTAL;
 
   mld.buffer = grub_malloc (MULTIBOOT_SEARCH);
   if (!mld.buffer)
@@ -258,6 +260,17 @@  grub_multiboot2_load (grub_file_t file, const char *filename)
 #endif
 	break;
 
+      case MULTIBOOT_HEADER_TAG_LOAD_TYPE:
+        load_type_tag = (struct multiboot_header_tag_load_type *) tag;
+        mld.load_type = load_type_tag->load_type;
+        if (mld.load_type >= MULTIBOOT_LOAD_TYPE_TOTAL)
+          {
+              grub_free (mld.buffer);
+              return grub_error (GRUB_ERR_UNKNOWN_OS,
+                                 "unsupported load type: %u", load_type_tag->load_type);
+          }
+        break;
+
       default:
 	if (! (tag->flags & MULTIBOOT_HEADER_TAG_OPTIONAL))
 	  {
@@ -268,14 +281,32 @@  grub_multiboot2_load (grub_file_t file, const char *filename)
 	break;
       }
 
-  if (addr_tag && !entry_specified && !(keep_bs && efi_entry_specified))
+  if (mld.load_type == MULTIBOOT_LOAD_TYPE_ADDRESS && !addr_tag)
+    {
+      grub_free (mld.buffer);
+      return grub_error (GRUB_ERR_UNKNOWN_OS,
+                         "load type address without address tag");
+    }
+  if (mld.load_type < MULTIBOOT_LOAD_TYPE_TOTAL &&
+      mld.load_type != MULTIBOOT_LOAD_TYPE_ADDRESS && addr_tag)
+    {
+      grub_free (mld.buffer);
+      return grub_error (GRUB_ERR_UNKNOWN_OS,
+                         "address tag specified but load type doesn't match");
+    }
+  if (mld.load_type == MULTIBOOT_LOAD_TYPE_TOTAL)
+    mld.load_type = addr_tag ? MULTIBOOT_LOAD_TYPE_ADDRESS : MULTIBOOT_LOAD_TYPE_ELF;
+
+  if (mld.load_type == MULTIBOOT_LOAD_TYPE_ADDRESS && !entry_specified &&
+      !(keep_bs && efi_entry_specified))
     {
       grub_free (mld.buffer);
       return grub_error (GRUB_ERR_UNKNOWN_OS,
 			 "load address tag without entry address tag");
     }
 
-  if (addr_tag)
+  switch (mld.load_type) {
+  case MULTIBOOT_LOAD_TYPE_ADDRESS:
     {
       grub_uint64_t load_addr = (addr_tag->load_addr + 1)
 	? addr_tag->load_addr : (addr_tag->header_addr
@@ -343,8 +374,9 @@  grub_multiboot2_load (grub_file_t file, const char *filename)
       if (addr_tag->bss_end_addr)
 	grub_memset ((grub_uint8_t *) source + load_size, 0,
 		     addr_tag->bss_end_addr - load_addr - load_size);
+      break;
     }
-  else
+  case MULTIBOOT_LOAD_TYPE_ELF:
     {
       mld.file = file;
       mld.filename = filename;
@@ -355,7 +387,14 @@  grub_multiboot2_load (grub_file_t file, const char *filename)
 	  grub_free (mld.buffer);
 	  return err;
 	}
+      break;
     }
+  case MULTIBOOT_LOAD_TYPE_PE:
+      grub_fatal ("Unsupported load type: %u\n", mld.load_type);
+  default:
+    /* should be impossible */
+    grub_fatal ("Unknown load type: %u\n", mld.load_type);
+  }
 
   load_base_addr = mld.load_base_addr;
 
diff --git a/include/grub/multiboot2.h b/include/grub/multiboot2.h
index b90aa6989674..1f6d4fc9e336 100644
--- a/include/grub/multiboot2.h
+++ b/include/grub/multiboot2.h
@@ -89,6 +89,7 @@  struct mbi_load_data
   grub_uint32_t link_base_addr;
   grub_uint32_t load_base_addr;
   int avoid_efi_boot_services;
+  grub_uint32_t load_type;
 };
 typedef struct mbi_load_data mbi_load_data_t;
 
diff --git a/include/multiboot2.h b/include/multiboot2.h
index a039aa0439aa..f4377bf7b394 100644
--- a/include/multiboot2.h
+++ b/include/multiboot2.h
@@ -74,6 +74,7 @@ 
 #define MULTIBOOT_HEADER_TAG_EFI_BS  7
 #define MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS_EFI64  9
 #define MULTIBOOT_HEADER_TAG_RELOCATABLE  10
+#define MULTIBOOT_HEADER_TAG_LOAD_TYPE  11
 
 #define MULTIBOOT2_ARCHITECTURE_I386  0
 #define MULTIBOOT2_ARCHITECTURE_MIPS32  4
@@ -178,6 +179,18 @@  struct multiboot_header_tag_relocatable
   multiboot_uint32_t preference;
 };
 
+struct multiboot_header_tag_load_type
+{
+  multiboot_uint16_t type;
+  multiboot_uint16_t flags;
+  multiboot_uint32_t size;
+#define MULTIBOOT_LOAD_TYPE_ADDRESS     0
+#define MULTIBOOT_LOAD_TYPE_ELF         1
+#define MULTIBOOT_LOAD_TYPE_PE          2
+#define MULTIBOOT_LOAD_TYPE_TOTAL       3
+  multiboot_uint32_t load_type;
+};
+
 struct multiboot_color
 {
   multiboot_uint8_t red;