Auto merge of #3219 - saethlin:map-failed, r=RalfJung

Return MAP_FAILED when mmap fails

I don't properly remember why we ended up with a hodgepodge of return values, but #3218 correctly points out that we are supposed to return `MAP_FAILED`. This should fix that return value and also add sufficient tests to prevent making a similar mistake.

Author: bors

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)?;

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

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() {