From 41c61e23db0d358383fdc8c95b1c046eceb4d147 Mon Sep 17 00:00:00 2001
From: Will Jhun <36206676+wjhun@users.noreply.github.com>
Date: Mon, 17 Jul 2023 10:45:41 -0400
Subject: [PATCH] stage2, uefi: allocate untyped buffers from tagged region

This addresses a long-standing bug in targets that use a rewind tag for
values (specifically the stage2 and uefi bootloaders), whereby an untyped
buffer allocation was being allocated without creating space for the tag
type. This could manifest in an incorrect tag type being read for a buffer. To
remedy this, an additional tagged region heap is created of type tag_unknown,
and untyped buffer values (currently only coming from decode_value() when
using an old TFS encoding for backward compatibility) are allocated from that
heap.
---
 platform/pc/boot/stage2.c |  5 +++++
 src/boot/boot.h           |  2 ++
 src/boot/uefi.c           |  5 +++++
 src/runtime/tuple.c       | 10 +++++++++-
 4 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/platform/pc/boot/stage2.c b/platform/pc/boot/stage2.c
index 3fa46e983..8dedffceb 100644
--- a/platform/pc/boot/stage2.c
+++ b/platform/pc/boot/stage2.c
@@ -328,6 +328,8 @@ static u64 stage2_allocator(heap h, bytes b)
     return result;
 }
 
+heap boot_buffer_heap;
+
 void centry()
 {
     working_heap.alloc = stage2_allocator;
@@ -341,6 +343,9 @@ void centry()
     init_symbols(allocate_tagged_region(&working_heap, tag_symbol), &working_heap);
     init_vectors(allocate_tagged_region(&working_heap, tag_vector), &working_heap);
     init_strings(allocate_tagged_region(&working_heap, tag_string), &working_heap);
+
+    /* given our rewind tag, all values must be tagged - even untyped buffers */
+    boot_buffer_heap = allocate_tagged_region(&working_heap, tag_unknown);
     init_sg(&working_heap);
     init_extra_prints();
     stage2_debug("%s\n", __func__);
diff --git a/src/boot/boot.h b/src/boot/boot.h
index 7c5c4b600..7bedeb367 100644
--- a/src/boot/boot.h
+++ b/src/boot/boot.h
@@ -54,3 +54,5 @@ static inline value_tag tagof(value v)
         return tag_integer;
     return *((u8 *)v-1);
 }
+
+extern heap boot_buffer_heap;
diff --git a/src/boot/uefi.c b/src/boot/uefi.c
index 8bfc62c0e..bbf6cda3d 100644
--- a/src/boot/uefi.c
+++ b/src/boot/uefi.c
@@ -237,6 +237,8 @@ closure_function(3, 2, void, uefi_bootfs_complete,
     }
 }
 
+heap boot_buffer_heap;
+
 efi_status efi_main(void *image_handle, efi_system_table system_table)
 {
     uefi_image_handle = image_handle;
@@ -256,6 +258,9 @@ efi_status efi_main(void *image_handle, efi_system_table system_table)
     init_symbols(allocate_tagged_region(&general, tag_symbol), &general);
     init_vectors(allocate_tagged_region(&general, tag_vector), &general);
     init_strings(allocate_tagged_region(&general, tag_string), &general);
+
+    /* given our rewind tag, all values must be tagged - even untyped buffers */
+    boot_buffer_heap = allocate_tagged_region(&general, tag_unknown);
     init_sg(&general);
     struct uefi_arch_options options;
     uefi_arch_setup(&general, &aligned_heap, &options);
diff --git a/src/runtime/tuple.c b/src/runtime/tuple.c
index 7561d731a..bb8d1d99b 100644
--- a/src/runtime/tuple.c
+++ b/src/runtime/tuple.c
@@ -381,7 +381,15 @@ value decode_value(heap h, table dictionary, buffer source, u64 *total,
             if (!old_encoding)
                 rprintf("%s: warning: untyped buffer, len %ld, offset %d: %X\n", __func__,
                         len, source->start, alloca_wrap_buffer(buffer_ref(source, 0), len));
-            b = allocate_buffer(h, len);
+
+            /* address a long-standing bug in bootloaders; untyped buffers must be tagged */
+            b = allocate_buffer(
+#ifdef BOOT
+                boot_buffer_heap,
+#else
+                h,
+#endif
+                len);
             assert(buffer_write(b, buffer_ref(source, 0), len));
             source->start += len;
         } else {