Stream: git-wasmtime

Topic: wasmtime / issue #9054 Access flags are lost when using `...

view this post on Zulip Wasmtime GitHub notifications bot (Jul 31 2024 at 22:16):

squeek502 added the bug label to Issue #9054.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 31 2024 at 22:16):

squeek502 opened issue #9054:

Test Case

(will upload .wasm file in a bit)

Steps to Reproduce

#include <sys/stat.h>
#include <sys/types.h>
#include <fcntl.h>
#include <unistd.h>

int main() {
    mkdirat(AT_FDCWD, "testdir", 0777);
    {
        // Opening with O_RDWR fails as expected if the dirfd is AT_FDCWD
        int dir = openat(AT_FDCWD, "testdir", O_RDWR);
        if (dir != -1) return 1;
    }

    // Open "testdir" for real now since we want to use it as the dirfd for openat
    int dir = openat(AT_FDCWD, "testdir", O_RDONLY|O_DIRECTORY);
    mkdirat(dir, "subdir", 0777);

    // Try to open the subdir with O_RDWR, this should fail with EISDIR but it succeeds
    // because the O_RDWR is lost and the openat2 syscall is called with O_RDONLY
    // instead
    int testdir = openat(dir, "subdir", O_RDWR);

    if (testdir != -1) return 1;
    return 0;
}

$ WASI_SDK=/home/ryan/Downloads/wasi-sdk-23.0-x86_64-linux
$ $WASI_SDK/bin/clang --sysroot=$WASI_SDK/share/wasi-sysroot open-dir-tmpdir-rw.c -o open-dir-tmpdir-rw-sdk-23.wasm
$ strace -e trace=openat2 wasmtime --dir=. open-dir-tmpdir-rw-sdk-23.wasm
openat2(3, "testdir", {flags=O_RDWR|O_LARGEFILE|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = -1 EISDIR (Is a directory)
openat2(3, "testdir", {flags=O_RDONLY|O_LARGEFILE|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 11
openat2(11, "subdir", {flags=O_RDONLY|O_LARGEFILE|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 12
+++ exited with 1 +++

Expected Results

The test program should exit with 0, and the last openat call should have O_RDWR set instead of O_RDONLY.

Actual Results

The test program exits with 1, since the last openat call loses the O_RDWR flag and erroneously sets O_RDONLY instead.

Versions and Environment

Wasmtime version or commit: wasi-sdk 23.0 and wasmtime 23.0.1

Operating system: Linux

Architecture: x86_64

Extra Info

This is a partial regression of a bug that was fixed by https://github.com/bytecodealliance/wasmtime/pull/6463. This bug is not present in wasmtime versions v9.0.1-v14.0.4

wasi-libc issue: https://github.com/WebAssembly/wasi-libc/issues/415

view this post on Zulip Wasmtime GitHub notifications bot (Aug 01 2024 at 00:35):

squeek502 edited issue #9054:

Test Case

open-dir-tmpdir-rw-sdk-23.zip

Steps to Reproduce

#include <sys/stat.h>
#include <sys/types.h>
#include <fcntl.h>
#include <unistd.h>

int main() {
    mkdirat(AT_FDCWD, "testdir", 0777);
    {
        // Opening with O_RDWR fails as expected if the dirfd is AT_FDCWD
        int dir = openat(AT_FDCWD, "testdir", O_RDWR);
        if (dir != -1) return 1;
    }

    // Open "testdir" for real now since we want to use it as the dirfd for openat
    int dir = openat(AT_FDCWD, "testdir", O_RDONLY|O_DIRECTORY);
    mkdirat(dir, "subdir", 0777);

    // Try to open the subdir with O_RDWR, this should fail with EISDIR but it succeeds
    // because the O_RDWR is lost and the openat2 syscall is called with O_RDONLY
    // instead
    int testdir = openat(dir, "subdir", O_RDWR);

    if (testdir != -1) return 1;
    return 0;
}

$ WASI_SDK=/home/ryan/Downloads/wasi-sdk-23.0-x86_64-linux
$ $WASI_SDK/bin/clang --sysroot=$WASI_SDK/share/wasi-sysroot open-dir-tmpdir-rw.c -o open-dir-tmpdir-rw-sdk-23.wasm
$ strace -e trace=openat2 wasmtime --dir=. open-dir-tmpdir-rw-sdk-23.wasm
openat2(3, "testdir", {flags=O_RDWR|O_LARGEFILE|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = -1 EISDIR (Is a directory)
openat2(3, "testdir", {flags=O_RDONLY|O_LARGEFILE|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 11
openat2(11, "subdir", {flags=O_RDONLY|O_LARGEFILE|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 12
+++ exited with 1 +++

Expected Results

The test program should exit with 0, and the last openat call should have O_RDWR set instead of O_RDONLY.

Actual Results

The test program exits with 1, since the last openat call loses the O_RDWR flag and erroneously sets O_RDONLY instead.

Versions and Environment

Wasmtime version or commit: wasi-sdk 23.0 and wasmtime 23.0.1

Operating system: Linux

Architecture: x86_64

Extra Info

This is a partial regression of a bug that was fixed by https://github.com/bytecodealliance/wasmtime/pull/6463. This bug is not present in wasmtime versions v9.0.1-v14.0.4

wasi-libc issue: https://github.com/WebAssembly/wasi-libc/issues/415

view this post on Zulip Wasmtime GitHub notifications bot (Aug 01 2024 at 00:35):

squeek502 edited issue #9054:

Test Case

open-dir-tmpdir-rw-sdk-23.zip

Steps to Reproduce

#include <sys/stat.h>
#include <sys/types.h>
#include <fcntl.h>
#include <unistd.h>

int main() {
    mkdirat(AT_FDCWD, "testdir", 0777);
    {
        // Opening with O_RDWR fails as expected if the dirfd is AT_FDCWD
        int dir = openat(AT_FDCWD, "testdir", O_RDWR);
        if (dir != -1) return 1;
    }

    // Open "testdir" for real now since we want to use it as the dirfd for openat
    int dir = openat(AT_FDCWD, "testdir", O_RDONLY|O_DIRECTORY);
    mkdirat(dir, "subdir", 0777);

    // Try to open the subdir with O_RDWR, this should fail with EISDIR but it succeeds
    // because the O_RDWR is lost and the openat2 syscall is called with O_RDONLY
    // instead
    int testdir = openat(dir, "subdir", O_RDWR);

    if (testdir != -1) return 1;
    return 0;
}

Built via:

$ WASI_SDK=/home/ryan/Downloads/wasi-sdk-23.0-x86_64-linux
$ $WASI_SDK/bin/clang --sysroot=$WASI_SDK/share/wasi-sysroot open-dir-tmpdir-rw.c -o open-dir-tmpdir-rw-sdk-23.wasm

Run with strace via:

$ strace -e trace=openat2 wasmtime --dir=. open-dir-tmpdir-rw-sdk-23.wasm
openat2(3, "testdir", {flags=O_RDWR|O_LARGEFILE|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = -1 EISDIR (Is a directory)
openat2(3, "testdir", {flags=O_RDONLY|O_LARGEFILE|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 11
openat2(11, "subdir", {flags=O_RDONLY|O_LARGEFILE|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 12
+++ exited with 1 +++

Expected Results

The test program should exit with 0, and the last openat call should have O_RDWR set instead of O_RDONLY.

Actual Results

The test program exits with 1, since the last openat call loses the O_RDWR flag and erroneously sets O_RDONLY instead.

Versions and Environment

Wasmtime version or commit: wasi-sdk 23.0 and wasmtime 23.0.1

Operating system: Linux

Architecture: x86_64

Extra Info

This is a partial regression of a bug that was fixed by https://github.com/bytecodealliance/wasmtime/pull/6463. This bug is not present in wasmtime versions v9.0.1-v14.0.4

wasi-libc issue: https://github.com/WebAssembly/wasi-libc/issues/415

view this post on Zulip Wasmtime GitHub notifications bot (Aug 01 2024 at 22:01):

alexcrichton commented on issue #9054:

Thanks for the report! It's not clear to me precisely where the fault lies when debugging this. Notes that follow are intended not as a fix for this issue but what I've found so far.

I've determined that this is "just a difference" between -Spreview2={y,n} which is wasi-common (old) vs wasmtime-wasi (new (ish)). The default, wasmtime-wasi, fails this test while wasi-common passes it. It appears due to this line where fd_fdstat_get on the dir variable in the program above is returning different results. This does not look related to path_open.

In wasmtime-wasi the fd_fdstat_get call returns

result=Ok(Fdstat { fs_filetype: Directory, fs_flags: Fdflags(0x0), fs_rights_base: Rights(... | FD_READ | ...), fs_rights_inheriting: Rights(... | FD_READ | ...) })

whereas wasi-common returns:

result=Ok(Fdstat { fs_filetype: Directory, fs_flags: Fdflags(0x0), fs_rights_base: Rights(...), fs_rights_inheriting: Rights(... | FD_READ | ... | FD_WRITE | ...) })

notably there's a difference in fs_rights_inheriting. This means that with wasi-common the request to write this directory is NOT masked out, hence the error. With wasmtime-wasi the request to write is masked out because fs_rights_inheriting doesn't have the write bit set.

@pchickey as the liberator of rights (hah!) are you able to recall the history around this? Is this something where we should update wasmtime-wasi to have the same fixed return value of fs_rights_inheriting? Or should we update wasi-libc to do something different about masking here?

Last updated: Nov 22 2024 at 16:03 UTC