Repository: tetratelabs/wazero
Pull request #1085 (2023-01-31 09:46:43)

wasi: prohibits closing pre-opens
See https://github.com/WebAssembly/wasi-testsuite/issues/50



Patch:
diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go
index 9d46ad33..1d628df1 100644
--- a/imports/wasi_snapshot_preview1/fs.go
+++ b/imports/wasi_snapshot_preview1/fs.go
@@ -48,6 +48,7 @@ var fdAllocate = stubFunction(
 //
 // The return value is ErrnoSuccess except the following error conditions:
 //   - ErrnoBadf: the fd was not open.
+//   - ErrnoNotsup: the fs was a pre-open
 //
 // Note: This is similar to `close` in POSIX.
 // See https://github.com/WebAssembly/WASI/blob/main/phases/snapshot/docs.md#fd_close
diff --git a/internal/sys/fs.go b/internal/sys/fs.go
index 2d361d36..6275dbff 100644
--- a/internal/sys/fs.go
+++ b/internal/sys/fs.go
@@ -259,7 +259,7 @@ func stdinReader(r io.Reader) *FileEntry {
 		r = eofReader{}
 	}
 	s := stdioStat(r, noopStdinStat)
-	return &FileEntry{IsPreopen: true, Name: noopStdinStat.Name(), File: &stdioFileReader{r: r, s: s}}
+	return &FileEntry{Name: noopStdinStat.Name(), File: &stdioFileReader{r: r, s: s}}
 }
 
 func stdioWriter(w io.Writer, defaultStat stdioFileInfo) *FileEntry {
@@ -267,7 +267,7 @@ func stdioWriter(w io.Writer, defaultStat stdioFileInfo) *FileEntry {
 		w = io.Discard
 	}
 	s := stdioStat(w, defaultStat)
-	return &FileEntry{IsPreopen: true, Name: s.Name(), File: &stdioFileWriter{w: w, s: s}}
+	return &FileEntry{Name: s.Name(), File: &stdioFileWriter{w: w, s: s}}
 }
 
 func stdioStat(f interface{}, defaultStat stdioFileInfo) fs.FileInfo {
@@ -350,6 +350,10 @@ func (c *FSContext) CloseFile(fd uint32) error {
 	f, ok := c.openedFiles.Lookup(fd)
 	if !ok {
 		return syscall.EBADF
+	} else if f.IsPreopen {
+		// WASI is the only user of pre-opens and wasi-testsuite disallows this
+		// See https://github.com/WebAssembly/wasi-testsuite/issues/50
+		return syscall.ENOTSUP
 	}
 	c.openedFiles.Delete(fd)
 	return f.File.Close()
diff --git a/internal/wasi_snapshot_preview1/errno.go b/internal/wasi_snapshot_preview1/errno.go
index 94b3faec..bfac8a9a 100644
--- a/internal/wasi_snapshot_preview1/errno.go
+++ b/internal/wasi_snapshot_preview1/errno.go
@@ -281,6 +281,8 @@ func ToErrno(err error) Errno {
 		return ErrnoNoent
 	case errors.Is(err, syscall.ENOSYS):
 		return ErrnoNosys
+	case errors.Is(err, syscall.ENOTSUP):
+		return ErrnoNotsup
 	case errors.Is(err, syscall.ENOTDIR):
 		return ErrnoNotdir
 	case errors.Is(err, syscall.EPERM), errors.Is(err, fs.ErrPermission):


Test patch:
diff --git a/imports/wasi_snapshot_preview1/fs_test.go b/imports/wasi_snapshot_preview1/fs_test.go
index e52a746d..bac6a2d3 100644
--- a/imports/wasi_snapshot_preview1/fs_test.go
+++ b/imports/wasi_snapshot_preview1/fs_test.go
@@ -80,6 +80,14 @@ func Test_fdClose(t *testing.T) {
 		require.Equal(t, `
 ==> wasi_snapshot_preview1.fd_close(fd=42)
 <== errno=EBADF
+`, "\n"+log.String())
+	})
+	log.Reset()
+	t.Run("ErrnoNotsup for a preopen", func(t *testing.T) {
+		requireErrno(t, ErrnoNotsup, mod, FdCloseName, uint64(sys.FdPreopen))
+		require.Equal(t, `
+==> wasi_snapshot_preview1.fd_close(fd=3)
+<== errno=ENOTSUP
 `, "\n"+log.String())
 	})
 }
diff --git a/internal/sys/fs_test.go b/internal/sys/fs_test.go
index 4a96c6a1..31cf6b23 100644
--- a/internal/sys/fs_test.go
+++ b/internal/sys/fs_test.go
@@ -20,9 +20,9 @@ import (
 var testCtx = context.WithValue(context.Background(), struct{}{}, "arbitrary")
 
 var (
-	noopStdin  = &FileEntry{IsPreopen: true, Name: "stdin", File: &stdioFileReader{r: eofReader{}, s: noopStdinStat}}
-	noopStdout = &FileEntry{IsPreopen: true, Name: "stdout", File: &stdioFileWriter{w: io.Discard, s: noopStdoutStat}}
-	noopStderr = &FileEntry{IsPreopen: true, Name: "stderr", File: &stdioFileWriter{w: io.Discard, s: noopStderrStat}}
+	noopStdin  = &FileEntry{Name: "stdin", File: &stdioFileReader{r: eofReader{}, s: noopStdinStat}}
+	noopStdout = &FileEntry{Name: "stdout", File: &stdioFileWriter{w: io.Discard, s: noopStdoutStat}}
+	noopStderr = &FileEntry{Name: "stderr", File: &stdioFileWriter{w: io.Discard, s: noopStderrStat}}
 )
 
 //go:embed testdata
@@ -94,6 +94,40 @@ func TestNewFSContext(t *testing.T) {
 	}
 }
 
+func TestFSContext_CloseFile(t *testing.T) {
+	embedFS, err := fs.Sub(testdata, "testdata")
+	require.NoError(t, err)
+	testFS := sysfs.Adapt(embedFS)
+
+	fsc, err := NewFSContext(nil, nil, nil, testFS)
+	require.NoError(t, err)
+	defer fsc.Close(testCtx)
+
+	fdToClose, err := fsc.OpenFile(testFS, "empty.txt", os.O_RDONLY, 0)
+	require.NoError(t, err)
+
+	fdToKeep, err := fsc.OpenFile(testFS, "test.txt", os.O_RDONLY, 0)
+	require.NoError(t, err)
+
+	// Close
+	require.NoError(t, fsc.CloseFile(fdToClose))
+
+	// Verify fdToClose is closed and removed from the opened FDs.
+	_, ok := fsc.LookupFile(fdToClose)
+	require.False(t, ok)
+
+	// Verify fdToKeep is not closed
+	_, ok = fsc.LookupFile(fdToKeep)
+	require.True(t, ok)
+
+	t.Run("EBADF for an invalid FD", func(t *testing.T) {
+		require.Equal(t, syscall.EBADF, fsc.CloseFile(42)) // 42 is an arbitrary invalid FD
+	})
+	t.Run("ENOTSUP for a preopen", func(t *testing.T) {
+		require.Equal(t, syscall.ENOTSUP, fsc.CloseFile(FdPreopen)) // 42 is an arbitrary invalid FD
+	})
+}
+
 func TestUnimplementedFSContext(t *testing.T) {
 	testFS, err := NewFSContext(nil, nil, nil, sysfs.UnimplementedFS{})
 	require.NoError(t, err)