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

path_open does not respect oflags::excl on directories #4791

Closed
yagehu opened this issue Jun 3, 2024 · 3 comments · Fixed by #4793
Closed

path_open does not respect oflags::excl on directories #4791

yagehu opened this issue Jun 3, 2024 · 3 comments · Fixed by #4793

Comments

@yagehu
Copy link
Contributor

yagehu commented Jun 3, 2024

Describe the bug

Calling path_open with oflags::creat and oflags::excl both set should fail if path points to an existing directory. Currently, Wasmer successfully open the directory and returns a valid fd.

$ wasmer -vV; rustc -vV
wasmer 4.3.1 (3930278 2024-05-31)
binary: wasmer-cli
commit-hash: 3930278b2e98528224ebf891f220aeacf2ec32db
commit-date: 2024-05-31
host: x86_64-apple-darwin
compiler: singlepass,cranelift
rustc 1.78.0 (9b00956e5 2024-04-29)
binary: rustc
commit-hash: 9b00956e56009bab2aa15d7bff10916599e3d6d6
commit-date: 2024-04-29
host: x86_64-apple-darwin
release: 1.78.0
LLVM version: 18.1.2

Steps to reproduce

Compile this Rust snippet with wasi v0.11. Create a directory dir before running. This snippet should fail, but Wasmer succeeds, returning a valid fd.

fn main() {
    unsafe {
        let fd = wasi::path_open(
            3,
            0,
            "dir",
            wasi::OFLAGS_CREAT | wasi::OFLAGS_EXCL,
            wasi::RIGHTS_FD_READ,
            0,
            0,
        )
        .unwrap();
        eprintln!("fd: {fd}");
    }
}
@yagehu
Copy link
Contributor Author

yagehu commented Jun 3, 2024

Sorry for the noise. I think I was on a bad commit.

@yagehu yagehu closed this as not planned Won't fix, can't repro, duplicate, stale Jun 3, 2024
@yagehu
Copy link
Contributor Author

yagehu commented Jun 3, 2024

I've reverted back to a good commit and this problem remains. I'll investigate and confirm with a wasi-fyi test case.

@yagehu yagehu reopened this Jun 3, 2024
@yagehu
Copy link
Contributor Author

yagehu commented Jun 3, 2024

Here's a wasi-fyi test that confirms the problem.

use std::ffi::CString;
use std::fs;

#[link(wasm_import_module = "wasi_snapshot_preview1")]
extern "C" {
    pub fn path_open(
        fd: i32,
        dirflags: i32,
        path: i32,
        path_len: i32,
        oflags: i32,
        fs_rights_base: i64,
        fs_rights_inheriting: i64,
        fdflags: i32,
        result_fd: i32,
    ) -> i32;
}

const ERRNO_SUCCESS: i32 = 0;
const ERRNO_EXIST: i32 = 20;
const OFLAGS_CREAT: i32 = 1;
const OFLAGS_EXCL: i32 = 4;
const RIGHTS_FD_READ: i64 = 2;
const RIGHTS_FD_WRITE: i64 = 64;

fn main() {
    unsafe {
        let fd = 5;
        let path0 = CString::new("fyi/fs_open_dir_excl.dir").unwrap();
        let path1 = CString::new("fyi/fs_open_dir_excl.dir/file").unwrap();
        let errno = path_open(
            fd,
            0,
            path0.as_ptr() as i32,
            path0.as_bytes().len() as i32,
            OFLAGS_CREAT | OFLAGS_EXCL,
            RIGHTS_FD_READ | RIGHTS_FD_WRITE,
            0,
            0,
            1024,
        );
        assert_eq!(
            errno, ERRNO_EXIST,
            "opening an existing directory with excl should fail"
        );

        let _ = fs::remove_file("/fyi/fs_open_dir_excl.dir/file");
        let errno = path_open(
            fd,
            0,
            path1.as_ptr() as i32,
            path1.as_bytes().len() as i32,
            OFLAGS_CREAT | OFLAGS_EXCL,
            RIGHTS_FD_READ | RIGHTS_FD_WRITE,
            0,
            0,
            1024,
        );
        assert_eq!(
            errno, ERRNO_SUCCESS,
            "opening a non-existing path with excl should succeed"
        );
    }
}

yagehu added a commit to yagehu/wasmer that referenced this issue Jun 3, 2024
This commit changes `path_open` to respect `oflags::excl` when opening
all types of files.  Per POSIX, the behavior of `O_EXCL` is unspecified
in the absence of `O_CREAT`, hence the addition of the `oflags::create`
check.

fixes wasmerio#4791
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

Successfully merging a pull request may close this issue.

1 participant