uefi-rs
uefi-rs copied to clipboard
uefi-rs APIs which want a well-aligned buffer but accept &mut [u8] are unsafe
If the user-provided slice doesn't actually match the expected alignment, undefined behavior will ensue.
FWIW, many of these buffer-wranging APIs actually want to receive an *mut [u8], because they are often meant to manipulate uninitialized memory and that concept doesn't exist at the Rust reference layer.
Would having some assertions for each such respective function ensure safety?
It would ensure safety in the presence of misaligned buffers (since [u8] itself has no such requirements), but using uninitialized memory as returned by e.g. UEFI's memory allocator would still be UB (although I don't think rustc can harmfully exploit this particular UB at the moment).
The unitialized memory UB can be solved by using &mut [MaybeUninit<u8>]
You can go a bit further and just use &mut [MaybeUninit<MemoryDescriptor>]
in memory_map()
.
@kotauskas the idea with &mut [MaybeUninit<u8>]
could work. The one with &mut [MaybeUninit<MemoryDescriptor>]
wouldn't necessarily, since the memory_map
API doesn't guarantee the returned memory descriptors are contiguous / occupy a fixed amount of spaced (you need to check each one to get the offset of the next.)
Currently memory_map
would panic if the buffer is not aligned, but it could just take a slice that is aligned
let buf = &mut buf[buf.as_ptr().align_offset(8)..];
Alternatively, a new type could be defined to represent a possibly uninitialized buffer with certain alignment requirements, e.g.:
#[repr(C, align(8))]
pub struct MemoryMapBuffer([MaybeUninit<u8>]);
A &mut MemoryMapBuffer
could then be constructed unsafely from a pointer and length, or safely from a byte slice. In the former case the caller is responsible for providing the correct alignment, in the latter case an incorrectly aligned slice can be handled by taking a subslice like @Soveu suggested.
Alternatively, a new type could be defined to represent a possibly uninitialized buffer with certain alignment requirements, e.g.:
#[repr(C, align(8))] pub struct MemoryMapBuffer([MaybeUninit<u8>]);
align(8)
also requires that length is a multiple of 8, so really this is just [u64]
(assuming u64 is aligned to 8)
Otherwise it is UB according to Miri
Minor nitpick: it's not that the length has to be a multiple of 8, because changing let newlen = newlen - (newlen % align);
to let newlen = newlen - 8;
stops Miri from complaining as well. I think the UB is caused by the Vec
allocation not backing the buffer's length rounded up to a multiple of 8 (although I don't understand why that would be UB). In any case, just using a slice is probably a better solution then.
It is UB, because size_of_val % align_of_val
must be 0
In this case, the wrapper struct has size in multiple of 8, even though the inner slice can have any size (so a multiple of 1)