Stream: git-wasmtime

Topic: wasmtime / PR #6348 Fix opening directories failing on Wi...


view this post on Zulip Wasmtime GitHub notifications bot (May 05 2023 at 16:25):

juamedgod requested jameysharp for a review on PR #6348.

view this post on Zulip Wasmtime GitHub notifications bot (May 05 2023 at 16:25):

juamedgod requested wasmtime-core-reviewers for a review on PR #6348.

view this post on Zulip Wasmtime GitHub notifications bot (May 05 2023 at 16:25):

juamedgod opened PR #6348 from juamedgod:windows-opendir-fix to bytecodealliance:main:

Description

We were testing the new wasmtime 8.0.1 on Windows and are getting "Permission denied" errors opening directories, while reading files under those directories work fine.

Test Case

The issue is reproducible using the below rust program (built as lister.wasm in the snippets) that directly calls opendir:

extern crate libc;
use std::ffi::CString;
use libc::{opendir, closedir, c_char};
use std::env;

fn main() {
    let args: Vec<String> = env::args().collect();
    if args.len() != 2 {
        println!("Usage: listdir <directory>");
        return;
    }
    let dir_name_cstring = CString::new(args[1].clone()).unwrap();
    unsafe {
        let dir_ptr = opendir(dir_name_cstring.as_ptr() as *const c_char);
        if dir_ptr.is_null() {
            println!("Error opening directory");
            return;
        } else {
            closedir(dir_ptr);
            println!("Opening directory worked");
        }
    }
}

Steps to Reproduce

Build a wasm module using the test case snippet, and invoke it pointing to a directory using wasmtime-8.0.1 on Windows:

$> c:\tests\wasmtime-v8.0.1-x86_64-windows\wasmtime.exe --mapdir /app::c:/tests/ c:\tests\lister.wasm -- /app
Error opening directory

The same command works when using 8.0.0:

$> c:\tests\wasmtime-v8.0.0-x86_64-windows\wasmtime.exe --mapdir /app::c:/tests/ c:\tests\lister.wasm -- /app
Opening directory worked

Versions and Environment

Wasmtime version or commit: wasmtime 8.0.1
Operating system: Windows 10
Architecture: x86_64

Extra details

The bug seems to be related to the changes in #6163. The code is failing here:

https://github.com/bytecodealliance/wasmtime/blob/main/crates/wasi-common/cap-std-sync/src/dir.rs#L93

Which was not previously applied to directories. That results on the directory being reopened which ends up calling Windows ReOpenFile, and that returns a "Access denied".

We have "fixed" the issue by moving the code to be executed only for files, but that may not be the best solution. It would be great of you could give us some tips if that is not the proper solution to improve the patch.

view this post on Zulip Wasmtime GitHub notifications bot (May 05 2023 at 16:53):

pchickey submitted PR review:

Thank you for finding and fixing this bug!

view this post on Zulip Wasmtime GitHub notifications bot (May 08 2023 at 17:01):

jameysharp submitted PR review:

This fix makes sense, and thank you for the new tests as well!

view this post on Zulip Wasmtime GitHub notifications bot (May 08 2023 at 17:40):

jameysharp merged PR #6348.


Last updated: Nov 22 2024 at 16:03 UTC