Skip to content

Commit

Permalink
show proper UB when making a too large allocation request
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Jun 17, 2024
1 parent 60a7200 commit 90970e3
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 26 deletions.
24 changes: 14 additions & 10 deletions src/alloc_bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
Expand All @@ -86,20 +88,22 @@ 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) };
alloc_bytes
}

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()
Expand Down
12 changes: 0 additions & 12 deletions src/shims/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
24 changes: 20 additions & 4 deletions src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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(
Expand Down
10 changes: 10 additions & 0 deletions tests/fail/alloc/too_large.rs
Original file line number Diff line number Diff line change
@@ -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
}
}
15 changes: 15 additions & 0 deletions tests/fail/alloc/too_large.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 90970e3

Please sign in to comment.