Thread: [tboot-devel] [PATCH 0 of 3] Use correct EFI memory map for Multiboot2 booting

Brought to you by: gwei3, hlukasz, mateuszmowka, mcamrom-intel, and 2 others

tboot-devel

[tboot-devel] [PATCH 0 of 3] Use correct EFI memory map for Multiboot2 booting

From: alex <thi...@gm...> - 2021-10-20 15:31:32

These patches allow tboot to boot an OS kernel/VMM using
Multiboot2 with a proper EFI Memory map.  This is relevant to booting
a non-Linux OS, such as Xen.

Issues fixed:

- The EFI-related code for marking reserved memory ranges did not create
  a new range if the requested range was outside all existing ranges.
  (This was different behavior than the related code for handling the E820 memory map)

- Although tboot previously updated its internal copy of the EFI Memory Map,
  the EFI Memory map used for Multiboot2 booting of the OS kernel/VMM was the original,
  lacking tboot's updates to it.

- There was no safeguard that prevented tboot's updated Multiboot2 content from
  overflowing the original area used for such.

[tboot-devel] [PATCH 1 of 3] make efi_memmap_reserve handle gaps like e820_protect_region already does

From: alex <thi...@gm...> - 2021-10-20 15:31:33

# HG changeset patch
# User Alex Olson <ale...@st...>
# Date 1634322959 18000
#      Fri Oct 15 13:35:59 2021 -0500
# Node ID 4eda9c0cf72d0fe0c217c251bdef2c5628e0c0a6
# Parent  5bf5c12411d3a4a7e0a552203b40bfe59d5c7789
make efi_memmap_reserve handle gaps like e820_protect_region already does

This revision corrects an inconsistency in the way efi_memmap_reserve
deals with requests that lie within a gap in the memory map.

In such cases, e820_protect_region() created a new range in the map,
however efi_memmap_reserve was doing nothing since its logic was
based on finding an overlapping range.

This revision makes efi_memmap_reserve add a new range.  Unlike
the e820 case, the EFI memory map have an "attributes" field as well
and it is somewhat ambigious how to define the attributes of a new map.
Since we are only using this for reserved ranges, the attributes are set to zero
for simplicity...

Signed-off-by: Alex Olson <ale...@st...>

diff -r 5bf5c12411d3 -r 4eda9c0cf72d tboot/common/efi_memmap.c
--- a/tboot/common/efi_memmap.c	Wed Sep 15 16:53:34 2021 +0200
+++ b/tboot/common/efi_memmap.c	Fri Oct 15 13:35:59 2021 -0500
@@ -144,6 +144,8 @@
  * Region has to be aligned to page size, function will round non-aligned
  * values. Base address is rounded down, length - up.
  * 
+ * If the specified region lies within a gap, a new region will be added
+ *
  * @param base   starting address
  * @param length length of region to reserve
  */
@@ -164,6 +166,7 @@
     uint64_t end = base + length;
     efi_mem_descr_t* desc = NULL;
     uint32_t i = 0;
+    bool in_range = false;
 
     while ((desc = efi_memmap_walk(desc)) != NULL) {
         uint64_t desc_base = desc->physical_start;
@@ -185,6 +188,9 @@
             goto cont;
         }
 
+        /* In all cases below, the current range is involved */
+        in_range = true;
+
         /* case 1: the current ram range is within the range:
            base, desc_base, desc_end, end */
         if ((base <= desc_base) && (desc_end <= end)) {
@@ -250,6 +256,15 @@
         ++i;
     }
 
+    /* Insert the new region */
+    if ( !in_range ) {
+
+        desc = efi_memmap_walk(NULL);
+        if( !insert_after_region(0, base, length, EFI_RESERVED_TYPE, 0) ) {
+            return false;
+        }
+    }
+
     return true;
 }

[tboot-devel] [PATCH 2 of 3] Replace EFI memory map in Multiboot2 info

From: alex <thi...@gm...> - 2021-10-20 15:31:37

# HG changeset patch
# User Alex Olson <ale...@st...>
# Date 1634669589 18000
#      Tue Oct 19 13:53:09 2021 -0500
# Node ID f3574795bf2ecbd0e717268f9cb9ccc8982f1861
# Parent  4eda9c0cf72d0fe0c217c251bdef2c5628e0c0a6
Replace EFI memory map in Multiboot2 info

tboot modifies both the e820 and EFI memory maps during its boot process,
but was only updating the Multiboot content's e820 map.  This meant
the unmodified EFI map was being passed even though it should have contained
reserved ranges specific to tboot.

This probably went unnoticied since the Linux kernel is not booted using
Multiboot (thus, was already receiving the updated EFI map on EFI systems).

This revision adds the update of the EFI Multiboot content, but has to make a
slight tradeoff as the combined expansion of the E820 and EFI memory maps
cannot be "paid" for with the memory savings from other Multiboot tag deletions.

Signed-off-by: Alex Olson <ale...@st...>

diff -r 4eda9c0cf72d -r f3574795bf2e tboot/common/efi_memmap.c
--- a/tboot/common/efi_memmap.c	Fri Oct 15 13:35:59 2021 -0500
+++ b/tboot/common/efi_memmap.c	Tue Oct 19 13:53:09 2021 -0500
@@ -372,4 +372,10 @@
     } else {
         return false;
     }
-}
\ No newline at end of file
+}
+
+
+bool efi_memmap_present(void)
+{
+    return efi_mmap_available;
+}
diff -r 4eda9c0cf72d -r f3574795bf2e tboot/common/loader.c
--- a/tboot/common/loader.c	Fri Oct 15 13:35:59 2021 -0500
+++ b/tboot/common/loader.c	Tue Oct 19 13:53:09 2021 -0500
@@ -331,6 +331,18 @@
     return true;
 }
 
+
+static bool remove_mb2_tag_by_type(loader_ctx *lctx, uint32_t tag_type)
+{
+    struct mb2_tag *start = next_mb2_tag(NULL);
+    struct mb2_tag *victim = find_mb2_tag_type(start, tag_type);
+
+    if (victim != NULL) {
+        return remove_mb2_tag(lctx,victim);
+    }
+    return false;
+}
+
 static bool
 grow_mb2_tag(loader_ctx *lctx, struct mb2_tag *which, uint32_t how_much)
 {
@@ -1401,11 +1413,22 @@
     }
 
     /* replace map in loader context with copy */
-    replace_e820_map(g_ldr_ctx);
+    if ( is_loader_launch_efi(g_ldr_ctx) && efi_memmap_present() ) {
+        /* for EFI, reclaim MB2 space by deleting the E820 map,
+           this ensures grow_mb2_tag() has enough slack available.
+           Due to the growth of each, there can only be one...
+         */
+        remove_mb2_tag_by_type(g_ldr_ctx, MB2_TAG_TYPE_MMAP);
+        replace_efi_map(g_ldr_ctx);
+    } else {
+        remove_mb2_tag_by_type(g_ldr_ctx, MB2_TAG_TYPE_EFI_MMAP);
+        replace_e820_map(g_ldr_ctx);
+    }
 
     if (get_tboot_dump_memmap()) {
         printk(TBOOT_DETA"adjusted e820 map:\n");
         print_e820_map();
+        efi_memmap_dump();
     }
 
     if ( !verify_loader_context(g_ldr_ctx) )
@@ -1907,6 +1930,47 @@
     return;
 }
 
+void
+replace_efi_map(loader_ctx *lctx)
+{
+    /* currently must be MBI type 2 */
+    if ( LOADER_CTX_BAD(lctx) || lctx->type == MB1_ONLY ){
+        return;
+    }
+
+    struct mb2_tag *start = (struct mb2_tag *)(lctx->addr + 8);
+    struct mb2_tag_efi_mmap *tag;
+    tag = (struct mb2_tag_efi_mmap *)find_mb2_tag_type(start, MB2_TAG_TYPE_EFI_MMAP);
+
+    if ( !tag ) {
+        printk(TBOOT_INFO"MB2 EFI map not found\n");
+        return;
+    }
+
+    const uint32_t old_mmap_size = tag->size - sizeof(struct mb2_tag_efi_mmap);
+    uint32_t new_descr_size=0;
+    uint32_t new_descr_vers=0;
+    uint32_t new_mmap_size=0;
+    void    *new_mmap;
+
+    new_mmap = (void *)efi_memmap_get_addr(&new_descr_size, &new_descr_vers, &new_mmap_size);
+
+    if ( old_mmap_size < new_mmap_size ) {
+        /* we have to grow */
+        if (false ==
+            grow_mb2_tag(lctx, (struct mb2_tag *)tag, (new_mmap_size-old_mmap_size))) {
+            printk(TBOOT_ERR"MB2 failed to grow EFI map tag\n");
+            return;
+        }
+    } else {
+        tag->size = sizeof(struct mb2_tag_efi_mmap) + new_mmap_size;
+    }
+    /* copy in new data */
+    tag->descr_size = new_descr_size;
+    tag->descr_vers = new_descr_vers;
+    tb_memcpy(tag->efi_mmap, new_mmap, new_mmap_size);
+}
+
 void print_loader_ctx(loader_ctx *lctx)
 {
     if (lctx->type != MB2_ONLY){
diff -r 4eda9c0cf72d -r f3574795bf2e tboot/include/efi_memmap.h
--- a/tboot/include/efi_memmap.h	Fri Oct 15 13:35:59 2021 -0500
+++ b/tboot/include/efi_memmap.h	Tue Oct 19 13:53:09 2021 -0500
@@ -98,4 +98,5 @@
                                       uint64_t *ram_base, uint64_t *ram_size);
 void efi_memmap_dump(void);
 
-#endif
\ No newline at end of file
+bool efi_memmap_present(void);
+#endif
diff -r 4eda9c0cf72d -r f3574795bf2e tboot/include/loader.h
--- a/tboot/include/loader.h	Fri Oct 15 13:35:59 2021 -0500
+++ b/tboot/include/loader.h	Tue Oct 19 13:53:09 2021 -0500
@@ -97,6 +97,7 @@
 extern void determine_loader_type(void *addr, uint32_t magic);
 extern unsigned long get_loader_ctx_end(loader_ctx *lctx);
 extern void replace_e820_map(loader_ctx *lctx);
+extern void replace_efi_map(loader_ctx *lctx);
 extern uint8_t *get_loader_rsdp(loader_ctx *lctx, uint32_t *length);
 extern bool is_loader_launch_efi(loader_ctx *lctx);
 extern bool get_loader_efi_ptr(loader_ctx *lctx, uint32_t *address,

[tboot-devel] [PATCH 3 of 3] Ensure that growth of Multiboot tags does not go beyond original area

From: alex <thi...@gm...> - 2021-10-20 15:31:38

# HG changeset patch
# User Alex Olson <ale...@st...>
# Date 1634323050 18000
#      Fri Oct 15 13:37:30 2021 -0500
# Node ID 06433602ffe048ceb946d538d51620ae8152c93d
# Parent  f3574795bf2ecbd0e717268f9cb9ccc8982f1861
Ensure that growth of Multiboot tags does not go beyond original area

The grow_mb2_tag() function can only rearrange Multiboot content
within the area originally allocated by the parent bootloader (Grub).

This revision ensures that boot will not continue if the expansion of
a tag would overflow the original area.

In practice, small expansions as a result of tboot added content
are normally "paid for" through tboot's removal of other tags.

Signed-off-by: Alex Olson <ale...@st...>

diff -r f3574795bf2e -r 06433602ffe0 tboot/common/loader.c
--- a/tboot/common/loader.c	Tue Oct 19 13:53:09 2021 -0500
+++ b/tboot/common/loader.c	Fri Oct 15 13:37:30 2021 -0500
@@ -375,6 +375,7 @@
     if (growth > slack){
         printk(TBOOT_ERR"YIKES!!! grow_mb2_tag slack %d < growth %d\n",
                slack, growth);
+        apply_policy(TB_ERR_FATAL);
     }
 
     /* now we copy down from the bottom, going up */