wasmtime / PR #6462 [9.0.0] fix Wasi rights system to wor... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #6462 [9.0.0] fix Wasi rights system to wor...

Wasmtime GitHub notifications bot (May 25 2023 at 21:59):

pchickey opened PR #6462 from bytecodealliance:pch/release-9-wasi-rights-read-write to bytecodealliance:release-9.0.0:

https://github.com/bytecodealliance/wasmtime/pull/6265 introduced a regression with programs using wasi-libc, reported at https://github.com/WebAssembly/wasi-libc/issues/415.

Wasi-libc read the rights of the base directory (using fd_fdstat_get) and used those to mask the rights requested to path_open. In 6265, I changed the behavior of fdstat_get to always report and empty set of rights. This means that Wasi-libc will always pass an empty set of rights to path_open, which is a problem because the FD_READ and FD_WRITE rights are how path_open determines if a descriptor is to be opened for reading, writing, or both.

The fix is as follows:

directories always return the full set of rights in fd_fdstat_get.

we record the access mode that a file is opened with, and use that to set the FD_READ and FD_WRITE bits in fs_rights_base for a file's fd_fdstat_get.

A test demonstrates the behavior of the fdstat rights bits, and that opening for reading, writing, or reading and writing behaves correctly when calling fd_read and fd_write

Wasmtime GitHub notifications bot (May 25 2023 at 21:59):

pchickey requested wasmtime-core-reviewers for a review on PR #6462.

Wasmtime GitHub notifications bot (May 25 2023 at 21:59):

pchickey requested alexcrichton for a review on PR #6462.

Wasmtime GitHub notifications bot (May 25 2023 at 22:14):

pchickey edited PR #6462:

https://github.com/bytecodealliance/wasmtime/pull/6265 introduced a regression with programs using wasi-libc, reported at https://github.com/WebAssembly/wasi-libc/issues/415.

Wasi-libc read the rights of the base directory (using fd_fdstat_get) and used those to mask the rights requested to path_open. In 6265, I changed the behavior of fdstat_get to always report and empty set of rights. This means that Wasi-libc will always pass an empty set of rights to path_open, which is a problem because the FD_READ and FD_WRITE rights are how path_open determines if a descriptor is to be opened for reading, writing, or both.

The fix is as follows:

directories always return the full set of rights in fd_fdstat_get.

we record the access mode that a file is opened with, and use that to set the FD_READ and FD_WRITE bits in fs_rights_base for a file's fd_fdstat_get.

A test demonstrates the behavior of the fdstat rights bits, and that opening for reading, writing, or reading and writing behaves correctly when calling fd_read and fd_write

This PR is just for the release-9.0.0 branch, I will work on upstreaming it to main but the situation there is slightly more complex because the test also needs to pass under the preview 2 implementation.

Wasmtime GitHub notifications bot (May 25 2023 at 22:45):

pchickey updated PR #6462.

Wasmtime GitHub notifications bot (May 25 2023 at 23:04):

pchickey requested sunfishcode for a review on PR #6462.

Wasmtime GitHub notifications bot (May 26 2023 at 00:23):

pchickey updated PR #6462.

Wasmtime GitHub notifications bot (May 26 2023 at 14:13):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (May 26 2023 at 15:20):

sunfishcode submitted PR review.

Wasmtime GitHub notifications bot (May 26 2023 at 16:50):

pchickey merged PR #6462.

Wasmtime GitHub notifications bot (Jul 23 2024 at 03:00):

squeek502 commented on PR #6462:

The bug that this fixed seems to have been reintroduced. With wasmtime 23.0.1 and wasi-sdk 23.0:

open-rw.c:

#include <stdio.h>
#include <stdlib.h>

int main() {
    // Note: using an `openat` call directly here with O_WRONLY or O_RDWR would have the same result
    FILE* file = fopen("testfile", "w");
    if (!file) return 1;
    fclose(file);
    return 0;
}

$ ls testfile
testfile
$ $WASI_SDK/bin/clang --sysroot=$WASI_SDK/share/wasi-sysroot open-rw.c -o open-rw-sdk.wasm
$ strace -e trace=openat2 wasmtime --dir=. open-rw-sdk.wasm
openat2(3, "testfile", {flags=O_RDONLY|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 5
+++ exited with 0 +++

wasmtime is calling openat2 with RDONLY, but "w" was passed as the access mode.

Wasmtime GitHub notifications bot (Jul 23 2024 at 03:16):

squeek502 edited a comment on PR #6462:

The bug that this fixed seems to have been reintroduced. With wasmtime 23.0.1 and wasi-sdk 23.0:

open-w.c:

#include <stdio.h>
#include <stdlib.h>

int main() {
    FILE* file = fopen("testfile", "w");
    if (!file) return 1;
    fclose(file);
    return 0;
}

$ ls testfile
testfile
$ $WASI_SDK/bin/clang --sysroot=$WASI_SDK/share/wasi-sysroot open-w.c -o open-w-sdk.wasm
$ strace -e trace=openat2 wasmtime --dir=. open-w-sdk.wasm
openat2(3, "testfile", {flags=O_RDONLY|O_LARGEFILE|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 11
+++ exited with 0 +++

wasmtime is calling openat2 with RDONLY, but "w" was passed as the access mode.

Wasmtime GitHub notifications bot (Jul 23 2024 at 03:26):

squeek502 deleted a comment on PR #6462:

The bug that this fixed seems to have been reintroduced. With wasmtime 23.0.1 and wasi-sdk 23.0:

open-w.c:

#include <stdio.h>
#include <stdlib.h>

int main() {
    FILE* file = fopen("testfile", "w");
    if (!file) return 1;
    fclose(file);
    return 0;
}

$ ls testfile
testfile
$ $WASI_SDK/bin/clang --sysroot=$WASI_SDK/share/wasi-sysroot open-w.c -o open-w-sdk.wasm
$ strace -e trace=openat2 wasmtime --dir=. open-w-sdk.wasm
openat2(3, "testfile", {flags=O_RDONLY|O_LARGEFILE|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 11
+++ exited with 0 +++

wasmtime is calling openat2 with RDONLY, but "w" was passed as the access mode.

Last updated: Dec 23 2024 at 12:05 UTC