Skip to content

fopen/openat does not seem to preserve access flags #415

Open
Open
fopen/openat does not seem to preserve access flags#415

Description

@squeek502
squeek502
opened on May 24, 2023

Tested on Linux with wasmtime v9.0.1 and wasi-sdk-20.0.

The following code (saved as open-rw.c) tries to open a file (that exists) for reading and writing:

#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;
}

EDIT: The above code originally used the invalid flags "rw", it has been corrected to use "w"

However, when using wasi-libc, the actual syscall (for me it's openat2) is being passed O_RDONLY instead of the expected O_WRONLY. Note: same applies to O_RDWR, O_RDONLY is still passed instead.

Compiled with wasi-sdk-20.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 +++

Note that access flags are preserved in the equivalent Zig code (unless -lc to link libc is used in which case wasi-libc is built/linked and the O_RDONLY behavior occurs)

const std = @import("std");

pub fn main() !void {
    var file = try std.fs.cwd().openFile("testfile", .{ .mode = .read_write });
    defer file.close();
}
$ zig build-exe -target wasm32-wasi open-rw.zig -femit-bin=open-rw-zig.wasm
$ strace -e trace=openat2 wasmtime --dir=. open-rw-zig.wasm
openat2(3, "testfile", {flags=O_RDWR|O_APPEND|O_NOFOLLOW|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 5
+++ exited with 0 +++

And are also preserved in the equivalent Rust code:

use std::fs::OpenOptions;

fn main() {
  let mut _file = OpenOptions::new().read(true).write(true).open("testfile").unwrap();
}
$ strace -e trace=openat2 wasmtime --dir=. open-rw-rust.wasm
openat2(3, "testfile", {flags=O_RDWR|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 5
+++ exited with 0 +++

Activity

  1. pchickey commented on May 24, 2023

    @pchickey
    pchickey
    on May 24, 2023
    Collaborator

    Can you please run wasmtime with the environment variable RUST_LOG=wasi_common=trace to provide additional debug information? That will help us determine whether this is a wasmtime or libc issue.

  2. jedisct1 commented on May 24, 2023

    @jedisct1
    jedisct1
    on May 24, 2023
    Member

  3. jedisct1 commented on May 24, 2023

    @jedisct1
    jedisct1
    on May 24, 2023
    Member

    wasmedge, wasm3 ,node and wasmer return an error, even when the libc is linked.

  4. jedisct1 commented on May 24, 2023

    @jedisct1
    jedisct1
    on May 24, 2023
    Member

    wasmtime 8.0.0 and all previous versions print error: IsDir

    The behavior changed in wasmtime 9.0.0.

    Log from wasmtime 8.0.0: wasmtime8.txt

  5. jedisct1 commented on May 24, 2023

    @jedisct1
    jedisct1
    on May 24, 2023
    Member

    diff between wasmtime 8 and wasmtime 9, with the same wasm file.

  6. pchickey commented on May 24, 2023

    @pchickey
    pchickey
    on May 24, 2023
    Collaborator

    Thanks. Can you please move this bug over to the wasmtime repository? This appears to be a bug over there, not in wasi-libc.

  7. jedisct1 commented on May 24, 2023

    @jedisct1
    jedisct1
    on May 24, 2023
    Member

    Something changed in wasmtime, but I'm not sure wasi-libc is not involved here.

    Looking at the traces, for the same code, this is what we see without wasi-libc:

    TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > wiggle abi; module="wasi_snapshot_preview1" function="path_open"

TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 >
fd=Fd(3) dirflags=(empty) path=*guest 0x100290/8 oflags=(empty)
fs_rights_base=FD_DATASYNC | FD_READ | FD_SEEK | FD_FDSTAT_SET_FLAGS |
FD_SYNC | FD_TELL | FD_WRITE | FD_ADVISE | FD_ALLOCATE |
FD_FILESTAT_GET | FD_FILESTAT_SET_SIZE | FD_FILESTAT_SET_TIMES
fs_rights_inheriting=(empty) fdflags=APPEND

TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > result=Err(Error { inner: Isdir })

    and with wasi-libc:

     TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 >
 result=Ok(Fdstat { fs_filetype: Directory, fs_flags: (empty),
 fs_rights_base: (empty), fs_rights_inheriting: (empty) })

 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > wiggle abi; module="wasi_snapshot_preview1" function="path_open"

 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 >
 fd=Fd(3) dirflags=SYMLINK_FOLLOW path=*guest 0xfef9c/8 oflags=(empty)
 fs_rights_base=(empty) fs_rights_inheriting=(empty) fdflags=(empty)

 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > result=Ok(Fd(4))

  8. jedisct1 commented on May 24, 2023

    @jedisct1
    jedisct1
    on May 24, 2023
    Member

    This is the wasmtime commit that changed the behavior: liberate wasi-common from Rights

  9. squeek502 commented on May 24, 2023

    @squeek502
    squeek502
    on May 24, 2023
    Author

    Just to make a clarification:

    wasmedge, wasm3 ,node and wasmer return an error, even when the libc is linked.

    wasmtime 8.0.0 and all previous versions print error: IsDir

    This is referring to how the problem was manifesting in the Zig test suite, where it'd try to open a directory with O_RDWR but EISDIR would not be returned. So, in @jedisct1's comments, 'return an error' is the correct behavior.

    The reproduction in the OP of this issue does not include that wrinkle and is instead focused on just the flags being passed to the syscall, since I realized the problem is more general than just opening directories for writing.

  10. pchickey commented on May 25, 2023

    @pchickey
    pchickey
    on May 25, 2023
    Collaborator

    This is a bug in Wasmtime 9.0.{0,1}. I have prepared a fix which we will release as 9.0.2.

    The behavior is the same across wasi-sdk 19 and 20, I did not test any further back but I believe it is the same for a long time.

    In the original test case given, this line:

        FILE* file = fopen("testfile", "rw");

    will actually open a file read-only. The mode string to open read-write is "r+".

    I made variants of that test for read-only (r), write after truncating (w), and read-write (r+) access, and am testing them here against the wasmtime executable built from bytecodealliance/wasmtime#6462

    [phickey@pch-tower:src/wasi_libc_415]% strace -e trace=openat2 ../wasmtime/target/release/wasmtime --disable-cache --dir . open-ro-sdk19.wasm
openat2(3, "testfile", {flags=O_RDONLY|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 5
+++ exited with 0 +++
[phickey@pch-tower:src/wasi_libc_415]% strace -e trace=openat2 ../wasmtime/target/release/wasmtime --disable-cache --dir . open-rw-sdk19.wasm
openat2(3, "testfile", {flags=O_RDWR|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 5
+++ exited with 0 +++
[phickey@pch-tower:src/wasi_libc_415]%
[phickey@pch-tower:src/wasi_libc_415]% strace -e trace=openat2 ../wasmtime/target/release/wasmtime --disable-cache --dir . open-ro-sdk20.wasm
openat2(3, "testfile", {flags=O_RDONLY|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 5
+++ exited with 0 +++
[phickey@pch-tower:src/wasi_libc_415]% strace -e trace=openat2 ../wasmtime/target/release/wasmtime --disable-cache --dir . open-rw-sdk20.wasm
openat2(3, "testfile", {flags=O_RDWR|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 5
+++ exited with 0 +++
[phickey@pch-tower:src/wasi_libc_415]% strace -e trace=openat2 ../wasmtime/target/release/wasmtime --disable-cache --dir . open-wo-sdk19.wasm
openat2(3, "testfile", {flags=O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, mode=0666, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 5
+++ exited with 0 +++
[phickey@pch-tower:src/wasi_libc_415]% strace -e trace=openat2 ../wasmtime/target/release/wasmtime --disable-cache --dir . open-wo-sdk20.wasm
openat2(3, "testfile", {flags=O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, mode=0666, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 5
+++ exited with 0 +++

  11. jedisct1 commented on May 25, 2023

    @jedisct1
    jedisct1
    on May 25, 2023
    Member

    Awesome!

    Thank you Pat!

  12. squeek502 commented on Jul 23, 2024

    @squeek502
    squeek502
    on Jul 23, 2024
    Author

    This seems to have regressed, but AFAICT only when using a non-AT_FDCWD dirfd in openat. Reproduction with wasi-sdk 23.0 and wasmtime 23.0.1:

    #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 +++

    The O_RDWR is lost when calling openat with an fd that's not AT_FDCWD.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions