-
- Notifications
You must be signed in to change notification settings - Fork 182
Description
Miri says that uefi::exts::allocate_buffer leads to undefined behavior. Here's an example in the playground that demonstrates the issue (click Tools > Miri). The relevant output:
error: Undefined Behavior: incorrect layout on deallocation: alloc1193 has size 64 and alignment 8, but gave size 64 and alignment 1 --> /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:107:14 | 107 | unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ incorrect layout on deallocation: alloc1193 has size 64 and alignment 8, but gave size 64 and alignment 1 | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information The issue is that an arbitrary Layout can be passed in, but the output type is Box<[u8]>. So when it comes time to free the Box, the Layout passed to dealloc always has an alignment of 1 rather than whatever alignment the input Layout had.
We could potentially make this API work by adding a type parameter and making the function return Box<[T]>. Then for example if you need alignment of 8 you use T=u64. However, I think a better option might be to just remove this API. Currently the only place this function is used within uefi-rs is uefi::proto::media::file::File::get_boxed_info. We can replace that usage easily enough. And although the function is part of the public API, I would guess it's not seeing much use -- it's generally more convenient to allocate with Vec, and you can always convert that to a Box with Vec::into_boxed_slice if you want.