show proper UB when making a too large allocation request

This commit is contained in:
Ralf Jung 2024-06-17 11:41:18 +02:00
parent 3f2c50c541
commit b8db9f0785
5 changed files with 59 additions and 26 deletions

View file

@ -64,17 +64,19 @@ impl MiriAllocBytes {
/// If `size == 0` we allocate using a different `alloc_layout` with `size = 1`, to ensure each allocation has a unique address.
/// Returns `Err(alloc_layout)` if the allocation function returns a `ptr` where `ptr.is_null()`.
fn alloc_with(
size: usize,
align: usize,
size: u64,
align: u64,
alloc_fn: impl FnOnce(Layout) -> *mut u8,
) -> Result<MiriAllocBytes, Layout> {
let layout = Layout::from_size_align(size, align).unwrap();
) -> Result<MiriAllocBytes, ()> {
let size = usize::try_from(size).map_err(|_| ())?;
let align = usize::try_from(align).map_err(|_| ())?;
let layout = Layout::from_size_align(size, align).map_err(|_| ())?;
// When size is 0 we allocate 1 byte anyway, to ensure each allocation has a unique address.
let alloc_layout =
if size == 0 { Layout::from_size_align(1, align).unwrap() } else { layout };
let ptr = alloc_fn(alloc_layout);
if ptr.is_null() {
Err(alloc_layout)
Err(())
} else {
// SAFETY: All `MiriAllocBytes` invariants are fulfilled.
Ok(Self { ptr, layout })
@ -86,11 +88,13 @@ impl AllocBytes for MiriAllocBytes {
fn from_bytes<'a>(slice: impl Into<Cow<'a, [u8]>>, align: Align) -> Self {
let slice = slice.into();
let size = slice.len();
let align = align.bytes_usize();
let align = align.bytes();
// SAFETY: `alloc_fn` will only be used with `size != 0`.
let alloc_fn = |layout| unsafe { alloc::alloc(layout) };
let alloc_bytes = MiriAllocBytes::alloc_with(size, align, alloc_fn)
.unwrap_or_else(|layout| alloc::handle_alloc_error(layout));
let alloc_bytes = MiriAllocBytes::alloc_with(size.try_into().unwrap(), align, alloc_fn)
.unwrap_or_else(|()| {
panic!("Miri ran out of memory: cannot create allocation of {size} bytes")
});
// SAFETY: `alloc_bytes.ptr` and `slice.as_ptr()` are non-null, properly aligned
// and valid for the `size`-many bytes to be copied.
unsafe { alloc_bytes.ptr.copy_from(slice.as_ptr(), size) };
@ -98,8 +102,8 @@ impl AllocBytes for MiriAllocBytes {
}
fn zeroed(size: Size, align: Align) -> Option<Self> {
let size = size.bytes_usize();
let align = align.bytes_usize();
let size = size.bytes();
let align = align.bytes();
// SAFETY: `alloc_fn` will only be used with `size != 0`.
let alloc_fn = |layout| unsafe { alloc::alloc_zeroed(layout) };
MiriAllocBytes::alloc_with(size, align, alloc_fn).ok()

View file

@ -5,18 +5,6 @@ use rustc_target::abi::{Align, Size};
use crate::*;
/// Check some basic requirements for this allocation request:
/// non-zero size, power-of-two alignment.
pub(super) fn check_alloc_request<'tcx>(size: u64, align: u64) -> InterpResult<'tcx> {
if size == 0 {
throw_ub_format!("creating allocation with size 0");
}
if !align.is_power_of_two() {
throw_ub_format!("creating allocation with non-power-of-two alignment {}", align);
}
Ok(())
}
impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
/// Returns the alignment that `malloc` would guarantee for requests of the given size.

View file

@ -12,7 +12,7 @@ use rustc_target::{
spec::abi::Abi,
};
use super::alloc::{check_alloc_request, EvalContextExt as _};
use super::alloc::EvalContextExt as _;
use super::backtrace::EvalContextExt as _;
use crate::*;
use helpers::{ToHost, ToSoft};
@ -204,6 +204,22 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {}
trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
/// Check some basic requirements for this allocation request:
/// non-zero size, power-of-two alignment.
fn check_rustc_alloc_request(&self, size: u64, align: u64) -> InterpResult<'tcx> {
let this = self.eval_context_ref();
if size == 0 {
throw_ub_format!("creating allocation with size 0");
}
if i128::from(size) > this.tcx.data_layout.pointer_size.signed_int_max() {
throw_ub_format!("creating an allocation larger than half the address space");
}
if !align.is_power_of_two() {
throw_ub_format!("creating allocation with non-power-of-two alignment {}", align);
}
Ok(())
}
fn emulate_foreign_item_inner(
&mut self,
link_name: Symbol,
@ -462,7 +478,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
let size = this.read_target_usize(size)?;
let align = this.read_target_usize(align)?;
check_alloc_request(size, align)?;
this.check_rustc_alloc_request(size, align)?;
let memory_kind = match link_name.as_str() {
"__rust_alloc" => MiriMemoryKind::Rust,
@ -496,7 +512,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
let size = this.read_target_usize(size)?;
let align = this.read_target_usize(align)?;
check_alloc_request(size, align)?;
this.check_rustc_alloc_request(size, align)?;
let ptr = this.allocate_ptr(
Size::from_bytes(size),
@ -560,7 +576,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
let new_size = this.read_target_usize(new_size)?;
// No need to check old_size; we anyway check that they match the allocation.
check_alloc_request(new_size, align)?;
this.check_rustc_alloc_request(new_size, align)?;
let align = Align::from_bytes(align).unwrap();
let new_ptr = this.reallocate_ptr(

View file

@ -0,0 +1,10 @@
extern "Rust" {
fn __rust_alloc(size: usize, align: usize) -> *mut u8;
}
fn main() {
let bytes = isize::MAX as usize + 1;
unsafe {
__rust_alloc(bytes, 1); //~ERROR: larger than half the address space
}
}

View file

@ -0,0 +1,15 @@
error: Undefined Behavior: creating an allocation larger than half the address space
--> $DIR/too_large.rs:LL:CC
|
LL | __rust_alloc(bytes, 1);
| ^^^^^^^^^^^^^^^^^^^^^^ creating an allocation larger than half the address space
|
= 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
= note: BACKTRACE:
= note: inside `main` at $DIR/too_large.rs:LL:CC
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
error: aborting due to 1 previous error