Security-harden the guest's memory allocator #726
Conversation
Extracted from #631
ceno_rt/src/allocator.rs
Outdated
| @@ -4,26 +4,34 @@ | |||
| use core::alloc::{GlobalAlloc, Layout}; | |||
|
|
|||
| struct SimpleAllocator { | |||
| next_alloc: usize, | |||
| next_alloc: *mut u8, | |||
There was a problem hiding this comment.
This type is more truthful, and simplifies our code.
ceno_rt/src/allocator.rs
Outdated
| } | ||
| core::hint::assert_unchecked(align.is_power_of_two()); | ||
| core::hint::assert_unchecked(align != 0); | ||
| heap_pos = heap_pos.add(heap_pos.align_offset(align)); |
There was a problem hiding this comment.
We use the proper Rust functions for working with pointers now. That makes both the type checker happier (when working with pointers instead of usize), and hopefully also clues in the optimiser a bit more.
But the latter would just be a welcome side effect, if it does happen.
ceno_rt/src/allocator.rs
Outdated
| next_alloc: 0xFFFF_FFFF, | ||
| }; | ||
|
|
||
| pub unsafe fn init_heap() { |
There was a problem hiding this comment.
We no longer need init_heap, because we can directly statically initialise to &raw mut _sheap,.
ceno_rt/src/allocator.rs
Outdated
| // (We could also return a null pointer, but only malicious programs would ever hit this.) | ||
| heap_pos = heap_pos.strict_add(layout.size()); |
There was a problem hiding this comment.
Alas, ptr type doesn't have strict_add, but add does nearly the same.
matthiasgoergens
changed the title
matthiasgoergens
changed the title
There was a problem hiding this comment.
my understanding of this PR:
previously, we also need to set the _sheap var somewhere(also linking time?), then allocator::init_heap() will use that value to overwrite the default 0xffffffff.
This PR seems a good pure code simplification change. Does it make anything really better or really worse? (if we are not talking about "more robust so in future there may be less bug in code modification", but just talk current code behavior)
To be really precise: we only ever care about the address of In this PR, I figured out that we can use Rust's static initialisation to set Then link time optimisation can kick in, and constant propagation can make
Current behaviour should be the same. The original motivation for this change was to make the standard library support simpler to implement: this way we have to worry less about what happens code wise before we run So to be honest, the motivation given in the PR description is a bit of a retro-fit. It's not wrong, but it's more of a side-benefit of a change we want to make for ease of standard library support. |
Here we harden the security of the guest's memory allocator: we no longer rely on manual initialisation code for its proper operation.
Because we remove code, it also simplifies the allocator. Hopefully making it easier to understand.
Extracted from #631