Fix sys_compat_mmap arithmetic bug

arca/src/table.rs

@@ -1,5 +1,10 @@
 use super::prelude::*;
 
+pub enum MapError<R: Runtime> {
+    Runtime(R::Error),
+    MapExists,
+}
+
 impl<R: Runtime> Table<R> {
     pub fn new(len: usize) -> Self {
         R::create_table(len)
@@ -25,7 +30,11 @@ impl<R: Runtime> Table<R> {
         Ok(())
     }
 
-    pub fn map(&mut self, address: usize, entry: Entry<R>) -> Result<Entry<R>, R::Error> {
+    // map behaves somewhat like Linux's MAP_FIXED_NOREPLACE (rejects an overlapping map with MapExists).
+    // Given the heterogeneity of page sizes, handling overlapping map requests involves
+    // a bunch of cases (e.g. attempt to map a 4 KiB page in the middle of an existing 2 MiB page, etc.)
+    pub fn map(&mut self, address: usize, entry: Entry<R>) -> Result<Entry<R>, MapError<R>> {
+        use MapError::*;
         if entry.is_empty() {
             return Ok(entry);
         }
@@ -34,26 +43,28 @@ impl<R: Runtime> Table<R> {
                 let mut embiggened = R::create_table(this.len() * 512);
                 embiggened.set(0, Entry::RWTable(this))?;
                 Ok(embiggened)
-            })?;
+            })
+            .map_err(Runtime)?;
             self.map(address, entry)?
         } else if entry.len() == self.len() / 512 {
             let shift = entry.len().ilog2();
             let index = address >> shift;
             assert!(index < 512);
-            self.set(index, entry)?
+            self.set(index, entry).map_err(Runtime)?
         } else {
             let shift = (self.len() / 512).ilog2();
             let index = (address >> shift) & 0x1ff;
             let offset = address & !(0x1ff << shift);
 
-            let mut smaller = match self.set(index, Entry::Null(0))? {
+            let mut smaller = match self.set(index, Entry::Null(0)).map_err(Runtime)? {
                 Entry::ROTable(table) => table,
                 Entry::RWTable(table) => table,
-                _ => R::create_table(self.len() / 512),
+                Entry::Null(_) => R::create_table(self.len() / 512),
+                Entry::ROPage(_) | Entry::RWPage(_) => return Err(MapExists),
             };
             assert!(self.len() > smaller.len());
             smaller.map(offset, entry)?;
-            self.set(index, Entry::RWTable(smaller))?
+            self.set(index, Entry::RWTable(smaller)).map_err(Runtime)?
         };
         Ok(result)
     }

fix/shell/src/lib.rs

@@ -117,3 +117,21 @@ pub fn main() -> ! {
     };
     os::exit(&result[..]);
 }
+
+const KERNEL_PAGE_SIZE: usize = 4096;
+
+/// Maps `length` bytes at `address` with the given `mode`.
+/// Panics if the kernel returns an error code or maps a different length than request
+///
+/// # Safety
+///
+/// [address] must refer to an unused region of memory at least `length` bytes long;
+/// there must be no Rust references pointing into that region.
+pub unsafe fn mmap(address: *mut c_void, length: usize, mode: u32) {
+    let expected = length.next_multiple_of(KERNEL_PAGE_SIZE);
+    let mapped = unsafe { arcane::arca_compat_mmap(address, length, mode) };
+    assert_eq!(
+        mapped, expected as i64,
+        "mmap of {length} bytes at {address:?} unexpectedly returned {mapped}"
+    );
+}

fix/shell/src/rt.rs

@@ -4,7 +4,7 @@
 
 use core::sync::atomic::{AtomicUsize, Ordering};
 
-use arcane::{__MODE_read_write, arca_compat_mmap};
+use arcane::__MODE_read_write;
 use user::error;
 
 include!(concat!(env!("OUT_DIR"), "/wasm_rt.rs"));
@@ -50,7 +50,7 @@ pub extern "C" fn wasm_rt_allocate_memory(
         assert!(max_pages <= (1u64 << 32) / PAGE_SIZE as u64);
         let data = ((1 << 32) * idx) as *mut u8;
         let size = initial_pages * PAGE_SIZE as u64;
-        arca_compat_mmap(data as *mut _, size as usize, __MODE_read_write);
+        crate::mmap(data as *mut _, size as usize, __MODE_read_write);
         memory.write(wasm_rt_memory_t {
             data,
             pages: initial_pages,
@@ -86,7 +86,7 @@ pub extern "C" fn wasm_rt_grow_memory(memory: *mut wasm_rt_memory_t, pages: u64)
     let start = unsafe { memory.data.byte_add(current as usize * PAGE_SIZE as usize) };
     let size = pages * PAGE_SIZE as u64;
     unsafe {
-        arca_compat_mmap(start as *mut _, size as usize, __MODE_read_write);
+        crate::mmap(start as *mut _, size as usize, __MODE_read_write);
         memory.pages += pages;
         memory.size += size;
     }
@@ -113,7 +113,7 @@ pub extern "C" fn wasm_rt_allocate_externref_table(
             max_elements = 1 << (32 - 5);
         }
         let data = ((1 << 32) * (64 + idx)) as *mut u8;
-        arca_compat_mmap(data as *mut _, (elements * 32) as usize, __MODE_read_write);
+        crate::mmap(data as *mut _, (elements * 32) as usize, __MODE_read_write);
         table.write(wasm_rt_externref_table_t {
             data: data as *mut _,
             size: elements,
@@ -137,7 +137,7 @@ pub extern "C" fn wasm_rt_grow_externref_table(
     let start = unsafe { table.data.byte_add(current as usize * 32) };
     let size = delta * 32;
     unsafe {
-        arca_compat_mmap(start as *mut _, size as usize, __MODE_read_write);
+        crate::mmap(start as *mut _, size as usize, __MODE_read_write);
         table.size += delta;
     }
     current
@@ -163,7 +163,7 @@ pub extern "C" fn wasm_rt_allocate_funcref_table(
             max_elements = 1 << (32 - 5);
         }
         let data = ((1 << 32) * (64 + 32 + idx)) as *mut u8;
-        arca_compat_mmap(
+        crate::mmap(
             data as *mut _,
             (elements as usize * core::mem::size_of::<wasm_rt_funcref_t>()) as usize,
             __MODE_read_write,

fix/shell/src/shell.rs

@@ -3,8 +3,7 @@ use arca::{Blob, Function, Table, Word};
 use arca::{Runtime as _, Tuple};
 use arcane::{
     __MODE_read_only, __MODE_read_write, __NR_length, __TYPE_table, arca_argument,
-    arca_blob_create, arca_blob_read, arca_compat_mmap, arca_entry, arca_mmap, arca_table_map,
-    arcad,
+    arca_blob_create, arca_blob_read, arca_entry, arca_mmap, arca_table_map, arcad,
 };
 
 use core::arch::x86_64::*;
@@ -52,7 +51,7 @@ pub unsafe fn fixpoint_attach_blob(addr: *mut c_void, handle: [u8; 32]) -> usize
     let len = fixpoint_len(handle);
 
     unsafe {
-        arca_compat_mmap(addr, len, __MODE_read_write);
+        crate::mmap(addr, len, __MODE_read_write);
         blob.read(0, core::slice::from_raw_parts_mut(addr as *mut u8, len));
     };
     // user::error::log_int("attached memory", len as u64);
@@ -86,7 +85,7 @@ pub unsafe fn fixpoint_attach_tree(addr: *mut c_void, handle: [u8; 32]) -> usize
     // user::error::log_int("attached tree", len as u64);
 
     unsafe {
-        arca_compat_mmap(addr, len * 32, __MODE_read_write);
+        crate::mmap(addr, len * 32, __MODE_read_write);
         let slice = core::slice::from_raw_parts_mut(addr as *mut u8, len * 32);
         tree.read(0, slice)
     };

kernel/src/cpu.rs

@@ -309,7 +309,10 @@ impl Cpu {
         };
         let table = Table::from(CowPage::Unique(pdpt));
         let mut table = arca::Table::from_inner(table);
-        let result = table.map(address, entry)?;
+        let result = table.map(address, entry).map_err(|e| match e {
+            arca::table::MapError::Runtime(e) => e,
+            arca::table::MapError::MapExists => crate::types::Error::MapError,
+        })?;
         match table.into_inner() {
             Table::Table512GB(page) => pml4.entry_mut(i_512gb).chain_unique(page.unique()),
             _ => todo!(),

kernel/src/types/function/syscall.rs

@@ -460,7 +460,9 @@ pub fn sys_compat_mmap(args: [u64; 6], arca: &mut LoadedArca) -> Result<usize> {
 
     let mut p = addr;
     while p < addr + len {
-        if p.is_multiple_of(Page1GB::SIZE) && len >= Page1GB::SIZE {
+        // Remaining number of bytes to mmap
+        let remaining = addr + len - p;
+        if p.is_multiple_of(Page1GB::SIZE) && remaining >= Page1GB::SIZE {
             if mode == arcane::__MODE_none {
                 let entry = Entry::Null(Page1GB::SIZE);
                 arca.cpu().map(p, entry).unwrap();
@@ -473,7 +475,7 @@ pub fn sys_compat_mmap(args: [u64; 6], arca: &mut LoadedArca) -> Result<usize> {
             p += Page1GB::SIZE;
             continue;
         }
-        if p.is_multiple_of(Page2MB::SIZE) && len >= Page2MB::SIZE {
+        if p.is_multiple_of(Page2MB::SIZE) && remaining >= Page2MB::SIZE {
             if mode == arcane::__MODE_none {
                 let entry = Entry::Null(Page2MB::SIZE);
                 arca.cpu().map(p, entry).unwrap();
@@ -500,7 +502,10 @@ pub fn sys_compat_mmap(args: [u64; 6], arca: &mut LoadedArca) -> Result<usize> {
         }
         panic!("unaligned mmap or bad size: {p:#x}+{len:#x}");
     }
-    Ok(p - addr)
+
+    let size_mapped = p - addr;
+    debug_assert_eq!(size_mapped, len.next_multiple_of(Page4KB::SIZE));
+    Ok(size_mapped)
 }
 
 pub fn sys_call_with_current_continuation(
@@ -670,6 +675,28 @@ impl From<crate::types::Error> for SyscallError {
             crate::types::Error::InvalidTableEntry(_) => SyscallError::BadArgument,
             crate::types::Error::InvalidIndex(_) => SyscallError::BadIndex,
             crate::types::Error::InvalidValue => SyscallError::BadArgument,
+            crate::types::Error::MapError => SyscallError::BadArgument,
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    // Verifies memory mapping rounds up to the nearest 4KB page without over allocation
+    #[test]
+    fn test_mmap_fits_request() {
+        let len: usize = Page2MB::SIZE + Page4KB::SIZE;
+        let mut cpu = CPU.borrow_mut();
+        let mut loaded_arca = Arca::new().load(&mut cpu);
+        let args = [0, len as u64, arcane::__MODE_read_write as u64, 0, 0, 0];
+        sys_compat_mmap(args, &mut loaded_arca).expect("mmap failed");
+        let mut arca = loaded_arca.unload();
+        let mappings = arca.mappings_mut();
+
+        assert_eq!(mappings.unmap(0).unwrap().len(), Page2MB::SIZE);
+        assert_eq!(mappings.unmap(Page2MB::SIZE).unwrap().len(), Page4KB::SIZE);
+        assert!(matches!(mappings.unmap(len), Some(Entry::Null(_))));
+    }
+}

kernel/src/types/runtime.rs

@@ -8,6 +8,7 @@ pub enum Error {
     InvalidTableEntry(super::Entry),
     InvalidIndex(usize),
     InvalidValue,
+    MapError,
 }
 
 impl arca::Runtime for Runtime {