From bebd6fb19f7898f1881cc5a97b7f75a532f90a99 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sun, 10 Dec 2023 12:23:40 -0500 Subject: [PATCH] Return MAP_FAILED when mmap fails --- src/tools/miri/src/shims/unix/linux/mem.rs | 2 +- src/tools/miri/src/shims/unix/mem.rs | 6 +- src/tools/miri/tests/pass-dep/shims/mmap.rs | 79 +++++++++++++++++++++ 3 files changed, 83 insertions(+), 4 deletions(-) diff --git a/src/tools/miri/src/shims/unix/linux/mem.rs b/src/tools/miri/src/shims/unix/linux/mem.rs index 026fd6ae5e7..d1b4416cb1d 100644 --- a/src/tools/miri/src/shims/unix/linux/mem.rs +++ b/src/tools/miri/src/shims/unix/linux/mem.rs @@ -38,7 +38,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if flags & this.eval_libc_i32("MREMAP_MAYMOVE") == 0 { // We only support MREMAP_MAYMOVE, so not passing the flag is just a failure this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?; - return Ok(Scalar::from_maybe_pointer(Pointer::null(), this)); + return Ok(this.eval_libc("MAP_FAILED")); } let old_address = Machine::ptr_from_addr_cast(this, old_address)?; diff --git a/src/tools/miri/src/shims/unix/mem.rs b/src/tools/miri/src/shims/unix/mem.rs index cf36d4b191c..5b84f047f49 100644 --- a/src/tools/miri/src/shims/unix/mem.rs +++ b/src/tools/miri/src/shims/unix/mem.rs @@ -48,11 +48,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // First, we do some basic argument validation as required by mmap if (flags & (map_private | map_shared)).count_ones() != 1 { this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?; - return Ok(Scalar::from_maybe_pointer(Pointer::null(), this)); + return Ok(this.eval_libc("MAP_FAILED")); } if length == 0 { this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?; - return Ok(Scalar::from_maybe_pointer(Pointer::null(), this)); + return Ok(this.eval_libc("MAP_FAILED")); } // If a user tries to map a file, we want to loudly inform them that this is not going @@ -72,7 +72,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // Miri doesn't support MAP_FIXED or any any protections other than PROT_READ|PROT_WRITE. if flags & map_fixed != 0 || prot != prot_read | prot_write { this.set_last_error(Scalar::from_i32(this.eval_libc_i32("ENOTSUP")))?; - return Ok(Scalar::from_maybe_pointer(Pointer::null(), this)); + return Ok(this.eval_libc("MAP_FAILED")); } // Miri does not support shared mappings, or any of the other extensions that for example diff --git a/src/tools/miri/tests/pass-dep/shims/mmap.rs b/src/tools/miri/tests/pass-dep/shims/mmap.rs index 03ffc3b3898..fefbed80fd2 100644 --- a/src/tools/miri/tests/pass-dep/shims/mmap.rs +++ b/src/tools/miri/tests/pass-dep/shims/mmap.rs @@ -2,6 +2,7 @@ //@compile-flags: -Zmiri-disable-isolation -Zmiri-permissive-provenance #![feature(strict_provenance)] +use std::io::Error; use std::{ptr, slice}; fn test_mmap() { @@ -32,6 +33,67 @@ fn test_mmap() { let just_an_address = ptr::invalid_mut(ptr.addr()); let res = unsafe { libc::munmap(just_an_address, page_size) }; assert_eq!(res, 0i32); + + // Test all of our error conditions + let ptr = unsafe { + libc::mmap( + ptr::null_mut(), + page_size, + libc::PROT_READ | libc::PROT_WRITE, + libc::MAP_PRIVATE | libc::MAP_SHARED, // Can't be both private and shared + -1, + 0, + ) + }; + assert_eq!(ptr, libc::MAP_FAILED); + assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL); + + let ptr = unsafe { + libc::mmap( + ptr::null_mut(), + 0, // Can't map no memory + libc::PROT_READ | libc::PROT_WRITE, + libc::MAP_PRIVATE | libc::MAP_ANONYMOUS, + -1, + 0, + ) + }; + assert_eq!(ptr, libc::MAP_FAILED); + assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL); + + let ptr = unsafe { + libc::mmap( + ptr::invalid_mut(page_size * 64), + page_size, + libc::PROT_READ | libc::PROT_WRITE, + // We don't support MAP_FIXED + libc::MAP_PRIVATE | libc::MAP_ANONYMOUS | libc::MAP_FIXED, + -1, + 0, + ) + }; + assert_eq!(ptr, libc::MAP_FAILED); + assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::ENOTSUP); + + // We don't support protections other than read+write + for prot in [libc::PROT_NONE, libc::PROT_EXEC, libc::PROT_READ, libc::PROT_WRITE] { + let ptr = unsafe { + libc::mmap( + ptr::null_mut(), + page_size, + prot, + libc::MAP_PRIVATE | libc::MAP_ANONYMOUS, + -1, + 0, + ) + }; + assert_eq!(ptr, libc::MAP_FAILED); + assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::ENOTSUP); + } + + 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); } #[cfg(target_os = "linux")] @@ -61,6 +123,23 @@ fn test_mremap() { let res = unsafe { libc::munmap(ptr, page_size * 2) }; assert_eq!(res, 0i32); + + // Test all of our error conditions + // Not aligned + let ptr = + unsafe { libc::mremap(ptr::invalid_mut(1), page_size, page_size, libc::MREMAP_MAYMOVE) }; + assert_eq!(ptr, libc::MAP_FAILED); + assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL); + + // Zero size + let ptr = unsafe { libc::mremap(ptr::null_mut(), page_size, 0, libc::MREMAP_MAYMOVE) }; + assert_eq!(ptr, libc::MAP_FAILED); + assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL); + + // Not setting MREMAP_MAYMOVE + let ptr = unsafe { libc::mremap(ptr::null_mut(), page_size, page_size, 0) }; + assert_eq!(ptr, libc::MAP_FAILED); + assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL); } fn main() {