Repository: tetratelabs/wazero (go)

Issue:
fclose <stdin>: Bad file descriptor
**Describe the bug**
I experiment with userland unix in Go and your project looks like a perfect match allowing me to run a unix utilities. After I build a few of them and tested them, I have noticed the `fclose <stdin>: Bad file descriptor` appeared. It happens for `os.Stdin`, `bytes.Buffer` or the `*io.PipeReader`. The `io.NopCloser` has no effect there. So I believe this is nothing to do with Go, but with the wasi integration of `fclose`.

**To Reproduce**
A small C reproducer

```c
#include <stdlib.h>
#include <stdio.h>

int main() {
    int ret = fclose(stdin);
    if (ret != 0) {
        perror("fclose <stdin>");
    }
    exit(ret);
}
```

To wasm - `zig cc --target=wasm32-wasi fclose.c -o fclose.wasm`

Run it via this program

```go
//go:embed testdata/fclose.wasm
var fcloseWasm []byte

func main() {
	ctx := context.Background()
	if err := run(ctx); err != nil {
		log.Fatal(err)
	}
}

func run(ctx context.Context) error {
	r := wazero.NewRuntime(ctx)
	defer r.Close(ctx)

	config := wazero.NewModuleConfig().
		WithStdin(io.NopCloser(os.Stdin)).
		WithStdout(os.Stdout).
		WithStderr(os.Stderr)
	wasi_snapshot_preview1.MustInstantiate(ctx, r)

	code, err := r.CompileModule(ctx, fcloseWasm)
	if err != nil {
		return err
	}

	if _, err = r.InstantiateModule(ctx, code, config.WithArgs("wasi")); err != nil {
		if exitErr, ok := err.(*sys.ExitError); ok && exitErr.ExitCode() != 0 {
			fmt.Fprintf(os.Stderr, "exit_code: %d\n", exitErr.ExitCode())
			return err
		} else if !ok {
			return err
		}
	}
	return nil
}
```

```sh
go run main.go 
fclose <stdin>: Bad file descriptor
exit_code: 4294967295
2022/12/21 09:13:07 module "" closed with exit_code(4294967295)
exit status 1
```

**Expected behavior**
The stdin is closed, no error message appear and exit code is 0.

**Screenshots**
If applicable, add screenshots to help explain your problem.

**Environment (please complete the relevant information):**
- Go version: go1.19.1 linux/amd64
- wazero Version: github.com/tetratelabs/wazero v1.0.0-pre.5
- Host architecture: amd64
- Runtime mode: compiler


Pull request:
wasi: allows closing stdio file descriptors
This allows wasm to close stdio file descriptors such as STDOUT(1). This will not close the underlying host resource as that would break a lot of folks doing logging to the console.

Thanks a lot to @vyskocilm for making this easy to identify and fix!

```bash
$ cat > fclose.c <<EOF
#include <stdlib.h>
#include <stdio.h>

int main() {
    int ret = fclose(stdin);
    if (ret != 0) {
        perror("fclose <stdin>");
    }
    exit(ret);
}
EOF

$ zig cc --target=wasm32-wasi fclose.c -o fclose.wasm
$ go build -o wazero ./cmd/wazero/...
$ ./wazero run fclose.wasm
```

Fixes #953


Patch:
diff --git a/internal/sys/fs.go b/internal/sys/fs.go
index ca56c4b5..b32e5fce 100644
--- a/internal/sys/fs.go
+++ b/internal/sys/fs.go
@@ -3,6 +3,7 @@ package sys
 import (
 	"context"
 	"errors"
+	"fmt"
 	"io"
 	"io/fs"
 	"math"
@@ -19,7 +20,6 @@ const (
 	FdStdin uint32 = iota
 	FdStdout
 	FdStderr
-
 	// FdRoot is the file descriptor of the root ("/") filesystem.
 	//
 	// # Why file descriptor 3?
@@ -34,6 +34,11 @@ const (
 	FdRoot
 )
 
+const (
+	modeDevice     = uint32(fs.ModeDevice | 0o640)
+	modeCharDevice = uint32(fs.ModeCharDevice | 0o640)
+)
+
 // EmptyFS is exported to special-case an empty file system.
 var EmptyFS = &emptyFS{}
 
@@ -71,6 +76,77 @@ func (emptyRootDir) ModTime() time.Time { return time.Unix(0, 0) }
 func (emptyRootDir) IsDir() bool        { return true }
 func (emptyRootDir) Sys() interface{}   { return nil }
 
+type stdioFileWriter struct {
+	w io.Writer
+	s fs.FileInfo
+}
+
+// Stat implements fs.File
+func (w *stdioFileWriter) Stat() (fs.FileInfo, error) { return w.s, nil }
+
+// Read implements fs.File
+func (w *stdioFileWriter) Read([]byte) (n int, err error) {
+	return // emulate os.Stdout which returns zero
+}
+
+// Write implements io.Writer
+func (w *stdioFileWriter) Write(p []byte) (n int, err error) {
+	return w.w.Write(p)
+}
+
+// Close implements fs.File
+func (w *stdioFileWriter) Close() error {
+	// Don't actually close the underlying file, as we didn't open it!
+	return nil
+}
+
+type stdioFileReader struct {
+	r io.Reader
+	s fs.FileInfo
+}
+
+// Stat implements fs.File
+func (r *stdioFileReader) Stat() (fs.FileInfo, error) { return r.s, nil }
+
+// Read implements fs.File
+func (r *stdioFileReader) Read(p []byte) (n int, err error) {
+	return r.r.Read(p)
+}
+
+// Close implements fs.File
+func (r *stdioFileReader) Close() error {
+	// Don't actually close the underlying file, as we didn't open it!
+	return nil
+}
+
+var (
+	noopStdinStat  = stdioFileInfo{FdStdin, modeDevice}
+	noopStdoutStat = stdioFileInfo{FdStdout, modeDevice}
+	noopStderrStat = stdioFileInfo{FdStderr, modeDevice}
+)
+
+// stdioFileInfo implements fs.FileInfo where index zero is the FD and one is the mode.
+type stdioFileInfo [2]uint32
+
+func (s stdioFileInfo) Name() string {
+	switch s[0] {
+	case FdStdin:
+		return "stdin"
+	case FdStdout:
+		return "stdout"
+	case FdStderr:
+		return "stderr"
+	default:
+		panic(fmt.Errorf("BUG: incorrect FD %d", s[0]))
+	}
+}
+
+func (stdioFileInfo) Size() int64         { return 0 }
+func (s stdioFileInfo) Mode() fs.FileMode { return fs.FileMode(s[1]) }
+func (stdioFileInfo) ModTime() time.Time  { return time.Unix(0, 0) }
+func (stdioFileInfo) IsDir() bool         { return false }
+func (stdioFileInfo) Sys() interface{}    { return nil }
+
 // FileEntry maps a path to an open file in a file system.
 type FileEntry struct {
 	// Name is the basename of the file, at the time it was opened. When the
@@ -102,10 +178,6 @@ type FSContext struct {
 	// fs is the root ("/") mount.
 	fs fs.FS
 
-	stdin                             io.Reader
-	stdout, stderr                    io.Writer
-	stdinStat, stdoutStat, stderrStat fs.FileInfo
-
 	// openedFiles is a map of file descriptor numbers (>=FdRoot) to open files
 	// (or directories) and defaults to empty.
 	// TODO: This is unguarded, so not goroutine-safe!
@@ -123,45 +195,14 @@ var errNotDir = errors.New("not a directory")
 // context can open files in that file system. Any error on opening "." is
 // returned.
 func NewFSContext(stdin io.Reader, stdout, stderr io.Writer, root fs.FS) (fsc *FSContext, err error) {
-	if stdin == nil {
-		stdin = eofReader{}
-	}
-
-	if stdout == nil {
-		stdout = io.Discard
-	}
-
-	if stderr == nil {
-		stderr = io.Discard
-	}
-
 	fsc = &FSContext{
-		stdin:       stdin,
-		stdout:      stdout,
-		stderr:      stderr,
-		fs:          root,
-		openedFiles: map[uint32]*FileEntry{},
-		lastFD:      FdStderr,
-	}
-
-	// Special case cached stat for stdio, notably using features that work in
-	// windows. This ensures tty detection can occur for python.
-	if stdin, ok := stdin.(*os.File); ok && platform.IsTerminal(stdin.Fd()) {
-		fsc.stdinStat = fileModeStat(fs.ModeCharDevice)
-	} else {
-		fsc.stdinStat = fileModeStat(fs.ModeDevice)
-	}
-
-	if stdout, ok := stdout.(*os.File); ok && platform.IsTerminal(stdout.Fd()) {
-		fsc.stdoutStat = fileModeStat(fs.ModeCharDevice)
-	} else {
-		fsc.stdoutStat = fileModeStat(fs.ModeDevice)
-	}
-
-	if stderr, ok := stderr.(*os.File); ok && platform.IsTerminal(stderr.Fd()) {
-		fsc.stderrStat = fileModeStat(fs.ModeCharDevice)
-	} else {
-		fsc.stderrStat = fileModeStat(fs.ModeDevice)
+		fs: root,
+		openedFiles: map[uint32]*FileEntry{
+			FdStdin:  stdinReader(stdin),
+			FdStdout: stdioWriter(stdout, noopStdoutStat),
+			FdStderr: stdioWriter(stderr, noopStderrStat),
+		},
+		lastFD: FdStderr,
 	}
 
 	if root == EmptyFS {
@@ -197,6 +238,29 @@ func NewFSContext(stdin io.Reader, stdout, stderr io.Writer, root fs.FS) (fsc *F
 	return fsc, nil
 }
 
+func stdinReader(r io.Reader) *FileEntry {
+	if r == nil {
+		r = eofReader{}
+	}
+	s := stdioStat(r, noopStdinStat)
+	return &FileEntry{Name: noopStdinStat.Name(), File: &stdioFileReader{r: r, s: s}}
+}
+
+func stdioWriter(w io.Writer, defaultStat stdioFileInfo) *FileEntry {
+	if w == nil {
+		w = io.Discard
+	}
+	s := stdioStat(w, defaultStat)
+	return &FileEntry{Name: defaultStat.Name(), File: &stdioFileWriter{w: w, s: s}}
+}
+
+func stdioStat(f interface{}, defaultStat stdioFileInfo) fs.FileInfo {
+	if f, ok := f.(*os.File); ok && platform.IsTerminal(f.Fd()) {
+		return stdioFileInfo{defaultStat[0], modeCharDevice}
+	}
+	return defaultStat
+}
+
 // nextFD gets the next file descriptor number in a goroutine safe way (monotonically) or zero if we ran out.
 // TODO: openedFiles is still not goroutine safe!
 // TODO: This can return zero if we ran out of file descriptors. A future change can optimize by re-using an FD pool.
@@ -214,14 +278,6 @@ func (c *FSContext) OpenedFile(fd uint32) (*FileEntry, bool) {
 }
 
 func (c *FSContext) StatFile(fd uint32) (fs.FileInfo, error) {
-	switch fd {
-	case FdStdin:
-		return c.stdinStat, nil
-	case FdStdout:
-		return c.stdoutStat, nil
-	case FdStderr:
-		return c.stderrStat, nil
-	}
 	f, ok := c.openedFiles[fd]
 	if !ok {
 		return nil, syscall.EBADF
@@ -284,34 +340,28 @@ func (c *FSContext) openFile(name string) (fs.File, error) {
 
 // FdWriter returns a valid writer for the given file descriptor or nil if syscall.EBADF.
 func (c *FSContext) FdWriter(fd uint32) io.Writer {
-	switch fd {
-	case FdStdout:
-		return c.stdout
-	case FdStderr:
-		return c.stderr
-	case FdRoot:
-		return nil // directory, not a writeable file.
-	default:
-		// Check to see if the file descriptor is available
-		if f, ok := c.openedFiles[fd]; !ok {
-			return nil
-		} else if writer, ok := f.File.(io.Writer); !ok {
-			// Go's syscall.Write also returns EBADF if the FD is present, but not writeable
-			return nil
-		} else {
-			return writer
-		}
+	// Check to see if the file descriptor is available
+	if f, ok := c.openedFiles[fd]; !ok {
+		return nil
+	} else if writer, ok := f.File.(io.Writer); !ok {
+		// Go's syscall.Write also returns EBADF if the FD is present, but not writeable
+		return nil
+	} else {
+		return writer
 	}
 }
 
 // FdReader returns a valid reader for the given file descriptor or nil if syscall.EBADF.
 func (c *FSContext) FdReader(fd uint32) io.Reader {
-	if fd == FdStdin {
-		return c.stdin
-	} else if fd == FdRoot {
+	switch fd {
+	case FdStdout, FdStderr:
+		return nil // writer, not a readable file.
+	case FdRoot:
 		return nil // directory, not a readable file.
-	} else if f, ok := c.openedFiles[fd]; !ok {
-		return nil
+	}
+
+	if f, ok := c.openedFiles[fd]; !ok {
+		return nil // TODO: could be a directory not a file.
 	} else {
 		return f.File
 	}


Test patch:
diff --git a/internal/sys/fs_test.go b/internal/sys/fs_test.go
index 1a4ed85e..4f2fb840 100644
--- a/internal/sys/fs_test.go
+++ b/internal/sys/fs_test.go
@@ -18,6 +18,12 @@ import (
 // testCtx is an arbitrary, non-default context. Non-nil also prevents linter errors.
 var testCtx = context.WithValue(context.Background(), struct{}{}, "arbitrary")
 
+var (
+	noopStdin  = &FileEntry{Name: noopStdinStat.Name(), File: &stdioFileReader{r: eofReader{}, s: noopStdinStat}}
+	noopStdout = &FileEntry{Name: noopStdoutStat.Name(), File: &stdioFileWriter{w: io.Discard, s: noopStdoutStat}}
+	noopStderr = &FileEntry{Name: noopStderrStat.Name(), File: &stdioFileWriter{w: io.Discard, s: noopStderrStat}}
+)
+
 //go:embed testdata
 var testdata embed.FS
 
@@ -88,15 +94,13 @@ func TestEmptyFSContext(t *testing.T) {
 	require.NoError(t, err)
 
 	expected := &FSContext{
-		stdin:       eofReader{},
-		stdinStat:   fileModeStat(fs.ModeDevice),
-		stdout:      io.Discard,
-		stdoutStat:  fileModeStat(fs.ModeDevice),
-		stderr:      io.Discard,
-		stderrStat:  fileModeStat(fs.ModeDevice),
-		fs:          EmptyFS,
-		openedFiles: map[uint32]*FileEntry{},
-		lastFD:      2,
+		fs: EmptyFS,
+		openedFiles: map[uint32]*FileEntry{
+			FdStdin:  noopStdin,
+			FdStdout: noopStdout,
+			FdStderr: noopStderr,
+		},
+		lastFD: FdStderr,
 	}
 
 	t.Run("OpenFile doesn't affect state", func(t *testing.T) {
@@ -108,12 +112,16 @@ func TestEmptyFSContext(t *testing.T) {
 		require.Equal(t, expected, testFS)
 	})
 
-	t.Run("Close doesn't affect state", func(t *testing.T) {
+	t.Run("Close closes", func(t *testing.T) {
 		err := testFS.Close(testCtx)
 		require.NoError(t, err)
 
-		// Ensure this didn't modify state
-		require.Equal(t, expected, testFS)
+		// Closes opened files
+		require.Equal(t, &FSContext{
+			fs:          EmptyFS,
+			openedFiles: map[uint32]*FileEntry{},
+			lastFD:      FdStderr,
+		}, testFS)
 	})
 }
 
@@ -180,11 +188,11 @@ func TestContext_Close(t *testing.T) {
 	fsc, err := NewFSContext(nil, nil, nil, testfs.FS{"foo": &testfs.File{}})
 	require.NoError(t, err)
 	// Verify base case
-	require.Equal(t, 1, len(fsc.openedFiles))
+	require.Equal(t, 1+FdRoot, uint32(len(fsc.openedFiles)))
 
 	_, err = fsc.OpenFile("foo")
 	require.NoError(t, err)
-	require.Equal(t, 2, len(fsc.openedFiles))
+	require.Equal(t, 2+FdRoot, uint32(len(fsc.openedFiles)))
 
 	// Closing should not err.
 	require.NoError(t, fsc.Close(testCtx))
diff --git a/internal/sys/sys_test.go b/internal/sys/sys_test.go
index 2300abe0..7c62bc87 100644
--- a/internal/sys/sys_test.go
+++ b/internal/sys/sys_test.go
@@ -2,7 +2,6 @@ package sys
 
 import (
 	"bytes"
-	"io"
 	"testing"
 	"time"
 
@@ -52,9 +51,12 @@ func TestDefaultSysContext(t *testing.T) {
 	require.Equal(t, platform.NewFakeRandSource(), sysCtx.RandSource())
 
 	expectedFS, _ := NewFSContext(nil, nil, nil, testfs.FS{})
-	require.Equal(t, eofReader{}, expectedFS.stdin)
-	require.Equal(t, io.Discard, expectedFS.stdout)
-	require.Equal(t, io.Discard, expectedFS.stderr)
+	require.Equal(t, map[uint32]*FileEntry{
+		FdStdin:  noopStdin,
+		FdStdout: noopStdout,
+		FdStderr: noopStderr,
+		FdRoot:   {Name: "/", File: emptyRootDir{}},
+	}, expectedFS.openedFiles)
 	require.Equal(t, expectedFS, sysCtx.FS())
 }