Skip to content

Commit

Permalink
stage2, uefi: allocate untyped buffers from tagged region
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
wjhun committed Aug 24, 2023
1 parent eb8be2f commit 41c61e2
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 1 deletion.
5 changes: 5 additions & 0 deletions platform/pc/boot/stage2.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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__);
Expand Down
2 changes: 2 additions & 0 deletions src/boot/boot.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,5 @@ static inline value_tag tagof(value v)
return tag_integer;
return *((u8 *)v-1);
}

extern heap boot_buffer_heap;
5 changes: 5 additions & 0 deletions src/boot/uefi.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
10 changes: 9 additions & 1 deletion src/runtime/tuple.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 41c61e2

Please sign in to comment.