rust-embedded-community / async-on-embedded

Apache License 2.0
123 stars 11 forks source link

Alloc is unsound #11

Open DutchGhost opened 3 years ago

DutchGhost commented 3 years ago

The alloc implementation is unsound.

To create an Alloc struct, Alloc::new() is called with a byte slice. Then, to allocate, Alloc::alloc is called. This seems to align to make an aligned allocation, but does not take into account that the start of the byte slice may be at any address.

For example, imagine a fresh Alloc struct:

Alloc {
    len: 1021,
    pos: 0,
    start: 3 // The *address* 3
}

Now imagine alloc::<u64>() is called, the important line here is:

let new_pos = round_up(self.pos, align);

This will set new_pos to 0, since round_up(0, 8) is 0. The line that actually calculates the final address is:

unsafe { &mut *(self.start.add(new_pos) as *mut MaybeUninit<T>) }

This will calculate the address of the allocation to be 3.add(0) = 3, and will hand out an &mut MaybeUninit<T> with the reference itself having the value 3. This is obviously not aligned, and therefore not sound.

To fix this, the following can be used:

fn next_power_of(n: usize, pow: usize) -> usize {
    let remain = n % pow;

    [n, n + (pow - remain)][(remain != 0) as usize]
}

impl Alloc {
    fn data_start_address(&self) -> usize {
        self.start as usize + self.pos
    }

    fn align_index_for<T>(&self) -> usize {
        let start_addr = self.data_start_address();
        let aligned_start = next_power_of(start_addr, core::mem::align_of::<T>());
        let aligned_index = aligned_start - self.start as usize;
        aligned_index
    }
}

This will start out by calculating the address that is free by adding self.pos to the base pointer (self.start). It then contininues to round that up to whatever the alignment of T requires. Lastly it substracts the base pointer from this value, effectively having calculated the offset such that base.add(offset) is aligned for T