Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mmap returns EBADF #2313

Closed
srwalter opened this issue Feb 14, 2024 · 2 comments
Closed

mmap returns EBADF #2313

srwalter opened this issue Feb 14, 2024 · 2 comments

Comments

@srwalter
Copy link

mmap fails to keep the file descriptor alive long enough to pass to mmap. The file descriptor gets closed and then mmap fails with EBADF. This shows the bug:

use std::fs::File;
use std::num::NonZeroUsize;

use nix::sys::mman::{MapFlags, ProtFlags};

fn main() {
    unsafe {
        let fd = File::open("/dev/mem").expect("open");
        nix::sys::mman::mmap(None, NonZeroUsize::new(4096).unwrap(), ProtFlags::PROT_READ,
            MapFlags::MAP_SHARED, Some(fd), 0).expect("mamp");
    }
}

Resulting output:

thread 'main' panicked at src/main.rs:10:48:
mamp: EBADF
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
@SteveLauC
Copy link
Member

mmap fails to keep the file descriptor alive long enough to pass to mmap.

// nix 0.27.1

pub unsafe fn mmap<F: AsFd>(
    addr: Option<NonZeroUsize>,
    length: NonZeroUsize,
    prot: ProtFlags,
    flags: MapFlags,
    f: Option<F>,
    offset: off_t,
) -> Result<*mut c_void> {
    let ptr =
        addr.map_or(std::ptr::null_mut(), |a| usize::from(a) as *mut c_void);

    // `f` dropped here, making `fd` an invalid file descriptor
    let fd = f.map(|f| f.as_fd().as_raw_fd()).unwrap_or(-1); 
    let ret =
        libc::mmap(ptr, length.into(), prot.bits(), flags.bits(), fd, offset);

This is right, if you move an owned file descriptor (like the std::fs::File in this issue) to it, then the fd will be closed before being passed to libc::mmap().

A workaround would be passing fd by reference:

        nix::sys::mman::mmap(None, NonZeroUsize::new(4096).unwrap(), ProtFlags::PROT_READ,
                             MapFlags::MAP_SHARED, Some(&fd), 0).expect("mamp");

But anyway, workarounds are just workarounds, let me file a PR to get this issue fixed so that the file descriptor can survive the underlying mmap() usage.

@SteveLauC
Copy link
Member

SteveLauC commented Feb 15, 2024

Just realized that this issue does not exist in the master branch as we have separated this API into

  1. mmap()
  2. mmap_anonymous()

so that the f argument in mmap() no longer takes an Option<F>, and thus eliminates the usage of Option::map(self), which would ensure that f will be alive within the whole function.

$ rg nix Cargo.toml
9:nix = { git = "https://github.com/nix-rust/nix" , features = ["mman"]}

$ cat src/main.rs           
use std::fs::File;
use std::num::NonZeroUsize;

use nix::sys::mman::{MapFlags, ProtFlags};

fn main() {
    unsafe {
        let fd = File::open("Cargo.toml").expect("open");
        nix::sys::mman::mmap(None, NonZeroUsize::new(4096).unwrap(), ProtFlags::PROT_READ,
                             MapFlags::MAP_SHARED, fd, 0).expect("mamp");
    }
}%    

$ cargo r -q

$ echo $?
0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants