Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security-harden the guest's memory allocator #726

Merged
merged 6 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 16 additions & 19 deletions ceno_rt/src/allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,34 @@
use core::alloc::{GlobalAlloc, Layout};

struct SimpleAllocator {
next_alloc: usize,
next_alloc: *mut u8,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type is more truthful, and simplifies our code.

}

extern "C" {
// The address of this variable is the start of the heap (growing upwards).
static mut _sheap: u8;
}

#[global_allocator]
static mut HEAP: SimpleAllocator = SimpleAllocator {
next_alloc: &raw mut _sheap,
};

unsafe impl GlobalAlloc for SimpleAllocator {
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
// SAFETY: Single threaded, so nothing else can touch this while we're working.
let mut heap_pos = HEAP.next_alloc;

let align = layout.align();
// `Layout` contract forbids making a `Layout` with align=0, or align not power of 2.
// So we can safely use subtraction and a mask to ensure alignment without worrying about UB.
let offset = heap_pos & (align - 1);
if offset != 0 {
heap_pos = heap_pos.strict_add(align.strict_sub(offset));
}
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));
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


let ptr = heap_pos as *mut u8;
let ptr = heap_pos;
// Panic on overflow. We don't want to wrap around, and overwrite stack etc.
// (We could also return a null pointer, but only malicious programs would ever hit this.)
heap_pos = heap_pos.strict_add(layout.size());
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alas, ptr type doesn't have strict_add, but add does nearly the same.

heap_pos = heap_pos.add(layout.size());

HEAP.next_alloc = heap_pos;
ptr
Expand All @@ -36,14 +44,3 @@ unsafe impl GlobalAlloc for SimpleAllocator {
/// Never deallocate.
unsafe fn dealloc(&self, _ptr: *mut u8, _layout: Layout) {}
}

#[global_allocator]
// We initialize `next_alloc` to 0xFFFF_FFFF to indicate that the heap has not been initialized.
// The value is chosen to make any premature allocation fail.
static mut HEAP: SimpleAllocator = SimpleAllocator {
next_alloc: 0xFFFF_FFFF,
};

pub unsafe fn init_heap() {
Copy link
Collaborator Author

@matthiasgoergens matthiasgoergens Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer need init_heap, because we can directly statically initialise to &raw mut _sheap,.

HEAP.next_alloc = core::ptr::from_ref::<u8>(&crate::_sheap).cast::<u8>() as usize;
}
11 changes: 3 additions & 8 deletions ceno_rt/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,14 @@ macro_rules! entry {
/// _start_rust is called by the assembly entry point and it calls the Rust main().
#[no_mangle]
unsafe extern "C" fn _start_rust() -> ! {
allocator::init_heap();
{
extern "C" {
fn bespoke_entrypoint();
}
bespoke_entrypoint();
extern "C" {
fn bespoke_entrypoint();
}
bespoke_entrypoint();
halt(0)
}

extern "C" {
// The address of this variable is the start of the stack (growing downwards).
static _stack_start: u8;
// The address of this variable is the start of the heap (growing upwards).
static _sheap: u8;
}
Loading