Auto merge of #3246 - saethlin:mmap-ices, r=RalfJung

Fix integer overflow ICEs from round_up_to_next_multiple_of

Turns out the standard library _does_ have the function we need. So I swapped us to using the checked version in mmap/munmap where we can return an error, and we're still using the ICEy version in SIMD.

I found one of the ICE cases by running this test: 765bcaab6e/src/mmap_allocator.rs (L195-L210)
This commit is contained in:
bors 2023-12-28 22:12:13 +00:00
commit 8bec0a50d2
5 changed files with 44 additions and 17 deletions

View file

@ -1233,11 +1233,3 @@ pub(crate) fn simd_element_to_bool(elem: ImmTy<'_, Provenance>) -> InterpResult<
_ => throw_ub_format!("each element of a SIMD mask must be all-0-bits or all-1-bits"),
})
}
// This looks like something that would be nice to have in the standard library...
pub(crate) fn round_to_next_multiple_of(x: u64, divisor: u64) -> u64 {
assert_ne!(divisor, 0);
// divisor is nonzero; multiplication cannot overflow since we just divided
#[allow(clippy::arithmetic_side_effects)]
return (x.checked_add(divisor - 1).unwrap() / divisor) * divisor;
}

View file

@ -11,6 +11,7 @@
#![feature(round_ties_even)]
#![feature(let_chains)]
#![feature(lint_reasons)]
#![feature(int_roundings)]
// Configure clippy and other lints
#![allow(
clippy::collapsible_else_if,

View file

@ -4,10 +4,7 @@ use rustc_middle::{mir, ty, ty::FloatTy};
use rustc_span::{sym, Symbol};
use rustc_target::abi::{Endian, HasDataLayout};
use crate::helpers::{
bool_to_simd_element, check_arg_count, round_to_next_multiple_of, simd_element_to_bool, ToHost,
ToSoft,
};
use crate::helpers::{bool_to_simd_element, check_arg_count, simd_element_to_bool, ToHost, ToSoft};
use crate::*;
#[derive(Copy, Clone)]
@ -407,7 +404,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let (yes, yes_len) = this.operand_to_simd(yes)?;
let (no, no_len) = this.operand_to_simd(no)?;
let (dest, dest_len) = this.place_to_simd(dest)?;
let bitmask_len = round_to_next_multiple_of(dest_len, 8);
let bitmask_len = dest_len.next_multiple_of(8);
// The mask must be an integer or an array.
assert!(
@ -453,7 +450,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
"bitmask" => {
let [op] = check_arg_count(args)?;
let (op, op_len) = this.operand_to_simd(op)?;
let bitmask_len = round_to_next_multiple_of(op_len, 8);
let bitmask_len = op_len.next_multiple_of(8);
// Returns either an unsigned integer or array of `u8`.
assert!(

View file

@ -14,7 +14,7 @@
//! munmap shim which would partily unmap a region of address space previously mapped by mmap will
//! report UB.
use crate::{helpers::round_to_next_multiple_of, *};
use crate::*;
use rustc_target::abi::Size;
impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
@ -96,7 +96,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}
let align = this.machine.page_align();
let map_length = round_to_next_multiple_of(length, this.machine.page_size);
let Some(map_length) = length.checked_next_multiple_of(this.machine.page_size) else {
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?;
return Ok(this.eval_libc("MAP_FAILED"));
};
if map_length > this.target_usize_max() {
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?;
return Ok(this.eval_libc("MAP_FAILED"));
}
let ptr =
this.allocate_ptr(Size::from_bytes(map_length), align, MiriMemoryKind::Mmap.into())?;
@ -129,7 +136,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
return Ok(Scalar::from_i32(-1));
}
let length = Size::from_bytes(round_to_next_multiple_of(length, this.machine.page_size));
let Some(length) = length.checked_next_multiple_of(this.machine.page_size) else {
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?;
return Ok(Scalar::from_i32(-1));
};
if length > this.target_usize_max() {
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?;
return Ok(this.eval_libc("MAP_FAILED"));
}
let length = Size::from_bytes(length);
this.deallocate_ptr(
addr,
Some((length, this.machine.page_align())),

View file

@ -90,9 +90,30 @@ fn test_mmap() {
assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::ENOTSUP);
}
// We report an error for mappings whose length cannot be rounded up to a multiple of
// the page size.
let ptr = unsafe {
libc::mmap(
ptr::null_mut(),
usize::MAX - 1,
libc::PROT_READ | libc::PROT_WRITE,
libc::MAP_PRIVATE | libc::MAP_ANONYMOUS,
-1,
0,
)
};
assert_eq!(ptr, libc::MAP_FAILED);
// We report an error when trying to munmap an address which is not a multiple of the page size
let res = unsafe { libc::munmap(ptr::invalid_mut(1), page_size) };
assert_eq!(res, -1);
assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL);
// We report an error when trying to munmap a length that cannot be rounded up to a multiple of
// the page size.
let res = unsafe { libc::munmap(ptr::invalid_mut(page_size), usize::MAX - 1) };
assert_eq!(res, -1);
assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL);
}
#[cfg(target_os = "linux")]