A simple mmap(2) wrapper

Question

I've written a wrapper for the mmap syscall. It's quite limited in functionality at the moment (there are lots of flags to support) but it's sufficient for mapping anonymous memory regions, which is what I need it for.

It already satisfies clippy, so I'm keen to understand some more advanced improvements that use Rust best-practices.

It's split into three files:

1. lib.rs containing an Error enum
2. mmap.rs containing an Mmap and MmapBuilder
3. syscall.rs containing wrapper for the mmap(2) syscall

lib.rs

#![allow(clippy::double_must_use)]
pub mod mmap;
pub mod syscall;
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub enum Error {
 System(i32),
 NullPointer(&'static str),
}
impl std::error::Error for Error {}
impl std::fmt::Display for Error {
 fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
 match self {
 Self::System(errno) => std::io::Error::from_raw_os_error(*errno).fmt(f),
 Self::NullPointer(msg) => write!(f, "null pointer: {msg}"),
 }
 }
}

mmap.rs

use crate::Error;
use std::ops::{BitOr, Deref, DerefMut};
use std::ptr::NonNull;
/// Represents a memory region that has been mapped with `mmap(2)`.
///
/// This can only be constructed through the use of [`MmapBuilder`].
///
/// Note that using this memory is inherently unsafe because it can concurrently
/// read and written by other processes; the borrow checker has no power here.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Mmap {
 addr: NonNull<u8>,
 len: usize,
}
impl Mmap {
 #[must_use]
 pub fn builder() -> MmapBuilder {
 MmapBuilder::new()
 }
 #[must_use]
 pub fn anon(len: usize) -> Result<Mmap, Error> {
 Self::builder().len(len).build()
 }
}
impl Drop for Mmap {
 fn drop(&mut self) {
 crate::syscall::munmap(self.addr, self.len).expect("failed to unmap");
 }
}
impl AsRef<[u8]> for Mmap {
 fn as_ref(&self) -> &[u8] {
 unsafe { std::slice::from_raw_parts(self.addr.as_ptr(), self.len) }
 }
}
impl AsMut<[u8]> for Mmap {
 fn as_mut(&mut self) -> &mut [u8] {
 unsafe { std::slice::from_raw_parts_mut(self.addr.as_ptr(), self.len) }
 }
}
impl Deref for Mmap {
 type Target = [u8];
 fn deref(&self) -> &Self::Target {
 self.as_ref()
 }
}
impl DerefMut for Mmap {
 fn deref_mut(&mut self) -> &mut Self::Target {
 self.as_mut()
 }
}
/// A builder for mmapped memory regions.
#[derive(Default, Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub struct MmapBuilder {
 len: usize,
 prot: i32,
 flags: i32,
 fd: i32,
 offset: i64,
 visibility: Visibility,
 addr: Option<NonNull<u8>>,
}
impl MmapBuilder {
 #[must_use]
 pub fn new() -> MmapBuilder {
 MmapBuilder::default()
 }
 #[must_use]
 pub fn len(mut self, len: usize) -> Self {
 self.len = len;
 self
 }
 #[must_use]
 pub fn addr(mut self, addr: Option<NonNull<u8>>) -> Self {
 self.addr = addr;
 self
 }
 #[must_use]
 pub fn visibility(mut self, visibility: Visibility) -> Self {
 self.visibility = visibility;
 self
 }
 #[must_use]
 pub fn protection<I: Into<i32>>(mut self, protection: I) -> Self {
 self.prot = protection.into();
 self
 }
 #[must_use]
 pub fn behavior<I: Into<i32>>(mut self, behavior: I) -> Self {
 self.flags = behavior.into();
 self
 }
 #[must_use]
 pub fn fd(mut self, fd: i32) -> Self {
 self.fd = fd;
 self
 }
 #[must_use]
 pub fn offset(mut self, offset: i64) -> Self {
 self.offset = offset;
 self
 }
 #[must_use]
 pub fn build(self) -> Result<Mmap, Error> {
 let addr = crate::syscall::mmap(
 self.addr,
 self.len,
 self.prot,
 self.flags | self.visibility,
 self.fd,
 self.offset,
 )?;
 Ok(Mmap {
 addr,
 len: self.len,
 })
 }
}
#[derive(Default, Hash, Debug, Copy, Clone, Eq, PartialEq)]
pub enum Visibility {
 #[default]
 Private,
 Shared,
}
impl From<Visibility> for i32 {
 fn from(value: Visibility) -> Self {
 match value {
 Visibility::Private => libc::MAP_PRIVATE,
 Visibility::Shared => libc::MAP_SHARED,
 }
 }
}
impl BitOr<Visibility> for Visibility {
 type Output = i32;
 fn bitor(self, rhs: Self) -> Self::Output {
 i32::from(self) | i32::from(rhs)
 }
}
impl BitOr<Visibility> for i32 {
 type Output = i32;
 fn bitor(self, rhs: Visibility) -> Self::Output {
 self | i32::from(rhs)
 }
}
#[derive(Default, Hash, Debug, Copy, Clone, Eq, PartialEq)]
pub enum Behavior {
 #[default]
 Anonymous,
}
impl From<Behavior> for i32 {
 fn from(value: Behavior) -> Self {
 match value {
 Behavior::Anonymous => libc::MAP_ANONYMOUS,
 }
 }
}
impl BitOr<Behavior> for Behavior {
 type Output = i32;
 fn bitor(self, rhs: Self) -> Self::Output {
 i32::from(self) | i32::from(rhs)
 }
}
impl BitOr<Behavior> for i32 {
 type Output = i32;
 fn bitor(self, rhs: Behavior) -> Self::Output {
 self | i32::from(rhs)
 }
}
/// Memory protection flags for mmap.
#[derive(Default, Hash, Debug, Copy, Clone, Eq, PartialEq)]
pub enum Protection {
 Exec,
 Read,
 Write,
 #[default]
 None,
}
impl From<Protection> for i32 {
 fn from(value: Protection) -> Self {
 match value {
 Protection::Exec => libc::PROT_EXEC,
 Protection::Read => libc::PROT_READ,
 Protection::Write => libc::PROT_WRITE,
 Protection::None => libc::PROT_NONE,
 }
 }
}
impl BitOr<Protection> for Protection {
 type Output = i32;
 fn bitor(self, rhs: Self) -> Self::Output {
 i32::from(self) | i32::from(rhs)
 }
}
impl BitOr<Protection> for i32 {
 type Output = i32;
 fn bitor(self, rhs: Protection) -> Self::Output {
 self | i32::from(rhs)
 }
}

syscall.rs

use crate::Error;
use std::ptr::NonNull;
pub(crate) fn mmap(
 addr: Option<NonNull<u8>>,
 len: usize,
 prot: i32,
 flags: i32,
 fd: i32,
 offset: i64,
) -> Result<NonNull<u8>, Error> {
 let ret = unsafe {
 let ptr = addr.map(NonNull::as_ptr).unwrap_or(std::ptr::null_mut());
 libc::mmap(ptr as *mut _, len, prot, flags, fd, offset)
 };
 if ret == libc::MAP_FAILED {
 Err(Error::System(errno()))
 } else {
 NonNull::new(ret as *mut _).ok_or(Error::NullPointer("mmap returned null"))
 }
}
pub(crate) fn munmap(addr: NonNull<u8>, len: usize) -> Result<(), Error> {
 let ret = unsafe { libc::munmap(addr.as_ptr() as *mut _, len) };
 if ret == -1 {
 Err(Error::System(errno()))
 } else {
 Ok(())
 }
}
#[must_use]
pub(crate) fn errno() -> i32 {
 std::io::Error::last_os_error()
 .raw_os_error()
 .unwrap_or_default()
}

• Toby Speight: Incorporating advice from an answer into the question violates the question-and-answer nature of this site. You could post improved code as a new question, as an answer, or as a link to an external site - as described in I improved my code based on the reviews. What next?. I have rolled back the edit, so the answers make sense again.
• cafce25: Definitely do add some testcases, and also whenever you have unsafe code, do run those testcases under cargo miri

Answer 1

This looks pretty good overall.

All I am really missing is tests. Some methods and functions may be covered by unit tests.

Some further, rather opinionated, nitpicks:

• I don't like the ret variables in mmap and munmap. It's a bad name and they're only ever used once.
• I also don't like the uncommented comparison to -1 in munmap. It's a magic number and may warrant a constant or a comment like // libc::munmap() failed. A good counter example of what I'd expect is your ret == libc::MAP_FAILED which is self-documenting.
• Protection::None looks like you could remove it from the enum and use Option<Protection> on first glance. However, this would make implementing the BitOr stuff a nightmare, though I don't see it being used anywhere anyway.

• haz: Thanks for the feedback! How would you test this sort of wrapper? Since mmap only does side effects, and those are a bit hard to check for (you could list /proc/self/maps but that could get complicated when tests are run concurrently) it seems difficult to do well
• haz: Answering my own follow-up comment: msync can be used to check if an address points to a mapped page.
• ChrisWue: @haz One possible way to test it is to exercise some use cases - mmap some memory, write to it, read the data back and check it's what you expect. If the test survives to that point it's probably a good indicator that things are working :)