aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSaleem Abdulrasool <compnerd@compnerd.org>2022-01-28 17:11:32 +0000
committerAndrew Waterman <andrew@sifive.com>2022-01-28 12:00:25 -0800
commit1d6f1bd0126596a17f97581346d8cea78cb3b5e9 (patch)
treee91320d2482dcf8108cb76ffee241c406ac0a844
parent387e54a5804653c4c4a22d958f05b4f91277a552 (diff)
downloadpk-1d6f1bd0126596a17f97581346d8cea78cb3b5e9.zip
pk-1d6f1bd0126596a17f97581346d8cea78cb3b5e9.tar.gz
pk-1d6f1bd0126596a17f97581346d8cea78cb3b5e9.tar.bz2
pk: thwart an attempt from the compiler to optimize
The memory manager maintains the first free page as the page after the `_end` synthetic emitted by the linker. This value is stored in a translation unit local variable. This value is only ever written to from `init_early_alloc` which is static and only ever invoked from `pk_vm_init`. Furthermore, the value that `first_free_page` is ever set to is computed as a rounding of the _address_ of `_end`. Because the address of the symbol cannot change during execution of a normal program, this is effectively a constant, making the computed value a "constant" which can be re-materialized. Now, with the knowledge that the value is effectively a constant that can be re-materialized and the fact that the value is ever written to at a single position, we can simply re-materialize the value if it was ever changed in `free_page_addr`. This will allow the 8-byte value to be truncated to 1-byte. Now, we can inline `__early_pgalloc_align`, and because the combination of `__early_alloc` and `__early_pgalloc_align` is small, we can inline that again at the two sites locally. This changes the `__augment_page_freelist` to re-materialize the constant when needed for the allocation. The re-materialization however uses a pc-relative addressing, which now computes a different value than expected - the address has become a VA rather than a PA. This results in the address computed by `free_page_addr` (which is the result of the `__early_pgalloc_align`) to be a virtual address after the relocation, which then propagates through `__early_alloc` to the value in `__augment_page_freelist`, which is then consumed by `__page_alloc`, which will treat the now VA as a PA and perform an additional translation to a VA. Mark the value as `volatile` to indicate that the value must be read at all points to thwart the size optimization of the compiler resulting in a mis-compilation resulting in the eventual invalid memory access during the `memset` that follows the allocation. Thanks to @nzmichaelh for the help in tracking this down!
-rw-r--r--pk/mmap.c3
1 files changed, 2 insertions, 1 deletions
diff --git a/pk/mmap.c b/pk/mmap.c
index 37aa87a..dd0fe59 100644
--- a/pk/mmap.c
+++ b/pk/mmap.c
@@ -554,7 +554,8 @@ static void init_early_alloc()
current.mmap_max = current.brk_max = user_size;
extern char _end;
- first_free_page = ROUNDUP((uintptr_t)&_end, RISCV_PGSIZE);
+ volatile uintptr_t last_static_addr = (uintptr_t)&_end;
+ first_free_page = ROUNDUP(last_static_addr, RISCV_PGSIZE);
free_pages = (mem_size - (first_free_page - MEM_START)) / RISCV_PGSIZE;
}