In wasmtime CLI, passing --tcplisten argument breaks --dir

[SteveSandersonMS]

Test Case

testcase.zip

Steps to Reproduce

1. Extract main.wasm from the test case zip file. Or compile it yourself using WASI SDK and the following source code, e.g. via ~/wasi-sdk/bin/clang main.c --sysroot ~/wasi-sdk/share/wasi-sysroot -o main.wasm if you have WASI SDK at ~/wasi-sdk.

#include <stdio.h>
#include <sys/types.h>
#include <dirent.h>

int main (void) {
  DIR *dp = opendir ("./");
  struct dirent *ep;

  if (dp != NULL) {
    while ((ep = readdir (dp)))
      printf("%s\n", ep->d_name);
    closedir(dp);
  }
  else
    printf("Couldn't open the directory\n");

  return 0;
}

2. Run wasmtime main.wasm --dir=. and observe it print a directory listing
3. Run wasmtime main.wasm --dir=. --tcplisten=127.0.0.1:9000 and observe it fail to do so (it will print Couldn't open the directory)

Expected Results

I expect that --tcplisten should not break the directory mapping.

Actual Results

Passing --tcplisten makes it behave as if you didn't pass --dir=..

Versions and Environment

Wasmtime version or commit: 0.35.1

Operating system: linux

Architecture: x86_64

Extra Info

Other than this, --tcplisten works brilliantly and I've been able to implement a nontrivial web server application using this flag and the new sock_accept API.

[sunfishcode]

CC @haraldh. It's possible this is related to how WASI programs discover the preopened sockets. Would it work to do the self.compute_preopen_dirs() before the self.compute_preopen_sockets(), so that the directories have lower file descriptor numbers and are found first?

This is an area where, in the future, Typed Main will help: instead of having WASI programs search through the file descriptor space to find preopens, we ideally want them provided explicitly.

[tschneidereit]

@haraldh is this something you might be able to look into? Addressing this looks like it'd help make --tcplisten quite a bit more useful :)

[haraldh]

yes, will do

[haraldh]

Ok, here is the analysis.

• socket gets fd 3
• directory gets fd 4
• wasi-libc/libc-bottom-half/sources/preopens.c:__wasilibc_populate_preopens() calls for every fd >= 3 fd_prestat_get() and internally registers directories. But if it encounters an error, like with fd 3 straight away, it bails out.

So with a WASI change like this:

diff --git a/phases/snapshot/witx/typenames.witx b/phases/snapshot/witx/typenames.witx
index 893e5b2..6aef907 100644
--- a/phases/snapshot/witx/typenames.witx
+++ b/phases/snapshot/witx/typenames.witx
@@ -730,6 +730,7 @@
   (enum (@witx tag u8)
     ;;; A pre-opened directory.
     $dir
+    $listen_socket
   )
 )
 
@@ -741,10 +742,19 @@
   )
 )
 
+;;; The contents of a $prestat when type is `preopentype::listen_socket`.
+(typename $prestat_listen_socket
+  (record
+    ;;; The length of the address string for use with `fd_prestat_socket_addr`.
+    (field $pr_addr_len $size)
+  )
+)
+
 ;;; Information about a pre-opened capability.
 (typename $prestat
   (union (@witx tag $preopentype)
     $prestat_dir
+    $prestat_listen_socket
   )
 )

we could fix wasmtime like this:

diff --git a/crates/wasi-common/src/snapshots/preview_0.rs b/crates/wasi-common/src/snapshots/preview_0.rs
index 96f86820b..8bdc2039a 100644
--- a/crates/wasi-common/src/snapshots/preview_0.rs
+++ b/crates/wasi-common/src/snapshots/preview_0.rs
@@ -198,6 +198,7 @@ impl From<snapshot1_types::Prestat> for types::Prestat {
     fn from(p: snapshot1_types::Prestat) -> types::Prestat {
         match p {
             snapshot1_types::Prestat::Dir(d) => types::Prestat::Dir(d.into()),
+            snapshot1_types::Prestat::ListenSocket(_) => todo!(),
         }
     }
 }
diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs
index 9c6f372d3..758b9da61 100644
--- a/crates/wasi-common/src/snapshots/preview_1.rs
+++ b/crates/wasi-common/src/snapshots/preview_1.rs
@@ -554,13 +554,26 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
 
     async fn fd_prestat_get(&mut self, fd: types::Fd) -> Result<types::Prestat, Error> {
         let table = self.table();
-        let dir_entry: &DirEntry = table.get(u32::from(fd)).map_err(|_| Error::badf())?;
-        if let Some(ref preopen) = dir_entry.preopen_path() {
-            let path_str = preopen.to_str().ok_or_else(|| Error::not_supported())?;
-            let pr_name_len = u32::try_from(path_str.as_bytes().len())?;
-            Ok(types::Prestat::Dir(types::PrestatDir { pr_name_len }))
+        if let Ok(dir_entry) = table.get::<DirEntry>(u32::from(fd)) {
+            if let Some(ref preopen) = dir_entry.preopen_path() {
+                let path_str = preopen.to_str().ok_or_else(|| Error::not_supported())?;
+                let pr_name_len = u32::try_from(path_str.as_bytes().len())?;
+                Ok(types::Prestat::Dir(types::PrestatDir { pr_name_len }))
+            } else {
+                Err(Error::not_supported().context("file is not a preopen"))
+            }
+        } else if let Ok(file_entry) = table.get_mut::<FileEntry>(u32::from(fd)) {
+            let fdstat = file_entry.get_fdstat().await?;
+            if matches!(fdstat.filetype, FileType::SocketStream) {
+                let pr_addr_len = 0; // TODO
+                Ok(types::Prestat::ListenSocket(types::PrestatListenSocket {
+                    pr_addr_len,
+                }))
+            } else {
+                Err(Error::not_supported().context("file is not a preopen"))
+            }
         } else {
-            Err(Error::not_supported().context("file is not a preopen"))
+            Err(Error::badf())
         }
     }

which would fix the code of this issue.

[haraldh]

The quick "band-aid" fix is https://github.com/bytecodealliance/wasmtime/pull/3995

[SteveSandersonMS]

Thanks for looking into this, @haraldh!

Just to clarify, how is application code meant to know which file descriptor numbers to pass to sock_accept? Is this a gap in the WASI spec, or does it say how they should be ordered or discovered? I could imagine two different WASI hosts might take different approaches (with the PR, wasmtime might put directory preopens first, but another runtime might put TCP listeners first).

Is there a recommended pattern of calls for application code to discover the available TCP listener file descriptors?

cc @sunfishcode

[bjorn3]

Is there a recommended pattern of calls for application code to discover the available TCP listener file descriptors?

Using the LISTEN_FDS env var. This env var is also used by systemd socket activation. According to man sd_listen_fds the first socket must be at fd 3 (so preopened directories must be after the preopened sockets, not before) and there must be $LISTEN_FDS sockets passed in.

[haraldh]

A combination of fd_prestat_get() for the directories and fd_fdstat_get() checking fs_filetype == SocketStream could be used without extending the WASI spec.

[SteveSandersonMS]

Thanks for the info! That's really useful.

preopened directories must be after the preopened sockets, not before

Does the "band-aid" fix (https://github.com/bytecodealliance/wasmtime/pull/3995) satisfy this condition? I did try out the patch given above and observed that it did still supply listener FDs starting from 3, even when there's also a directory preopen.

[haraldh]

Is there a recommended pattern of calls for application code to discover the available TCP listener file descriptors?

Using the LISTEN_FDS env var. This env var is also used by systemd socket activation. According to man sd_listen_fds the first socket must be at fd 3 (so preopened directories must be after the preopened sockets, not before) and there must be $LISTEN_FDS sockets passed in.

Or we have to increment the LISTEN_FDS env var by the number of the preopened directories (and also LISTEN_FDNAMES).

[haraldh]

Thanks for the info! That's really useful.

preopened directories must be after the preopened sockets, not before

Does the "band-aid" fix (#3995) satisfy this condition? I did try out the patch given above and observed that it did still supply listener FDs starting from 3, even when there's also a directory preopen.

The "band-aid" fix violates this condition... The fix with the WASI spec change satisfies it.

[npmccallum]

@bjorn3 One problem with LISTEN_FDS is that it is only used for fd >= 3. What if an application has fewer file descriptors?

[bjorn3]

Fds 0, 1 and 2 are reserved for stdin, stdout and stderr respectively. Wasi doesn't allow them to be absent AFAIK.

[haraldh]

Anyway, to get into this situation, you have to deliberately use the new command line features or the new WasiCtx methods and as I said,

A combination of fd_prestat_get() for the directories and fd_fdstat_get() checking fs_filetype == SocketStream could be used without extending the WASI spec.

So, instead of DIR *dp = opendir ("./"); one would use DIR *dp = fdopendir (fd);

[npmccallum]

@bjorn3 I don't see stdio in the snapshot at all.

[bjorn3]

What snapshot?

[npmccallum]

@bjorn3 https://github.com/WebAssembly/WASI/blob/main/phases/snapshot/docs.md

[bjorn3]

Both the wasi impl in wasmtime and wasi-libc assume the first three fds are stdio:

https://github.com/bytecodealliance/wasmtime/blob/76f7cde6734ecb5b89ac122a0f91af3a7a7a04a1/crates/wasi-common/src/ctx.rs#L35-L37

https://github.com/WebAssembly/wasi-libc/blob/079adff840032c3455eb1cb34dc9ceaa0b2bfc0c/libc-bottom-half/sources/preopens.c#L213-L215

[sdeleuze]

Hi, thanks for raising this since I hit the same problem when implementing a web server that serve static files. I am wondering if there is a consensus on how to fix it? Could be the case based on this discussion above but I am not 100% sure.

@bjorn3 said:

According to man sd_listen_fds the first socket must be at fd 3 (so preopened directories must be after the preopened sockets, not before)

Then @haraldh said:

A combination of fd_prestat_get() for the directories and fd_fdstat_get() checking fs_filetype == SocketStream could be used without extending the WASI spec.

So is it correct to assume that it is possible to fix this issue without extending the WASI spec, in a compliant way with man sd_listen_fds (preopened directories must be after the preopened sockets) just by refining how Wasmtime handles this?

Happy to try crafting a PR with proper guidance.

[npmccallum]

Both the wasi impl in wasmtime and wasi-libc assume the first three fds are stdio:

https://github.com/bytecodealliance/wasmtime/blob/76f7cde6734ecb5b89ac122a0f91af3a7a7a04a1/crates/wasi-common/src/ctx.rs#L35-L37

https://github.com/WebAssembly/wasi-libc/blob/079adff840032c3455eb1cb34dc9ceaa0b2bfc0c/libc-bottom-half/sources/preopens.c#L213-L215

But it isn't in the standard. Implementations can assume anything they want. It doesn't make it standard behavior.

[bjorn3]

Anything that wants to use wasi-libc implicitly assumes this. Assemblyscript also assumes it (https://github.com/AssemblyScript/assemblyscript/blob/d884ac8032b2bfa0caf26d4dc11d99c5a9543c13/std/assembly/wasi/index.ts#L57) Nodejs also uses 0, 1 and 2 for stdin, stdout and stderr by default. Because of this IMO it should just be added to the actual wasi standard as it is already de-facto standard, just not de-jure.

[npmccallum]

@bjorn3 But this is an example of a host feature "bleeding" into the spec. In Enarx, the host stdio is untrusted and shouldn't be connected to the encrypted guest wasm instance. I do not thing we should presume that just because everyone assumes it that it should be added to WASI uncritically.

[bjorn3]

It isn't required that the host stdio is connected to the guest stdio. The guest can have /dev/null or a file on which every operations fails as stdio if there is no stdio to pass through.

[sunfishcode]

WASI is in the process of moving to interface types and Typed Main, at which point I expect we'll completely overhaul the way file descriptors are passed into programs. The accept feature here is about enabling certain functionality with minimal changes to the current infrastructure.

Within the current infrastructure, fds 0,1,2 are effectively reserved for stdin, stdout, stderr. wasi-libc assumes this. This isn't how POSIX would do things, or how Linux would do them, but it's how the current infrastructure works, so the way to make things work with minimal changes is just to teach everything that 0,1,2 are reserved for stdin, stdout, and stderr.

[tiran]

Hi, is there any progress on this issue? I would like to use --tcplisten to experiment with sockets from CPython WASI port. For now Python also requires --mapdir or --dir to mount Python's stdlib.

[squillace]

@sunfishcode is there any resolution we might work toward here? This continues to break a few things.

[squillace]

it appears that https://github.com/bytecodealliance/wasmtime/pull/3995 should fix it, but...

[alexcrichton]

@squillace is it possible to use components instead and use the wasi:sockets proposal? Not much thought has gone into trying to backfill the socket APIs of preview1 with the implementations of preview2 because it was assumed that there weren't all that many users of preview1 sockets. This may be possible to get working somewhat, but in general it'd probably be best to start riding the preview2/component train if possible.

[squillace]

we'll have a look to see whether the wasi-sockets proposal is ready for freddy. In general, there'll be a choice that language runtimes make here, so for them this is a fork in the road, so to speak....

[thaystg]

Hi, @alexcrichton , currently we use the sock_accept importing the function like this in our C code:

__attribute__((import_module("wasi_snapshot_preview1"))) __attribute__((import_name("sock_accept"))) int sock_accept(int fd, int fdflags, int* result_ptr);

How should I use components to replace it? Is there any documentation that I can follow?