wasmer icon indicating copy to clipboard operation
wasmer copied to clipboard

Fix preopened current directory not working

Open yagehu opened this issue 1 year ago • 5 comments

This commit fixes a bug where creating a file with an implicit relative path returns 0 indicating success, but no file is created on the host. Here, implicit relative means a relative path that does not begin with ./. For example, calling open("somefile", O_CREAT | O_WRONLY) will try to create a file called somefile in the current working directory.

The fix is to change the current directory if the user has explicitly mounted any host directory to . in guest. In this case, the user's intention is clear: the current directory is the mounted host directory.

fixes https://github.com/wasmerio/wasmer/issues/4361

yagehu avatar Dec 18 '23 19:12 yagehu

@theduke Gentle ping. I think this is a reasonable fix for a common use case not working on Wasmer. Specifically, something like:

#include <fcntl.h>
#include <stdio.h>

int main(void) {
  int fd = open("somefile", O_CREAT | O_WRONLY);\
  if (fd == -1) {
    perror("open");
    return 1;
  }

  return 0;
}

yagehu avatar Dec 22 '23 19:12 yagehu

This needs a bit more discussion between @theduke and @syrusakbary (me). Stay tuned, and thanks for opening the PR @yagehu (and the patience!)

syrusakbary avatar Jan 09 '24 10:01 syrusakbary

This needs a bit more discussion between @theduke and @syrusakbary (me). Stay tuned, and thanks for opening the PR @yagehu (and the patience!)

Hi @syrusakbary, any update? Just to jolt your memory, the motivation for this PR is that a simple path_open() with an implicit relative path (a relative path without a leading ./) does not work with Wasmer while all other runtimes can handle this behavior. In a contrived C example:

open("somefile", O_CREAT | O_RDWR)

should create a file somefile at the current directory.

For context, I'm differential fuzzing various wasm runtimes at the raw WASI level (bypassing wasi-libc). Wasmer is the only runtime where I need to apply this patch so the file system behavior is consistent with other runtimes.

I can update this PR. There are probably much better ways to make Wasmer's behavior make sense both with and without wasi-libc.

yagehu avatar Apr 12 '24 22:04 yagehu

Hey @yagehu , most of the tests are failing on this branch, which is why I'm hesitant to merge atm

syrusakbary avatar Apr 30 '24 13:04 syrusakbary

Hey @yagehu , most of the tests are failing on this branch, which is why I'm hesitant to merge atm

To be clear, I was not asking to merge this PR as is since a lot has changed since my initial PR. I'll update this PR if there's interest in aligning Wasmer path_open behavior with other runtimes.

yagehu avatar Apr 30 '24 16:04 yagehu