mleku/wazero
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

wasi: enforce fd_preXXX functions only work on pre-opened files (#919)

Browse Source 
At the moment, the only pre-open support we have is the file system
itself (root a.k.a. / or file-descriptor 3). We may in the future add
the ability to pre-open sockets, but in any case, this is where we are
today.

This change hardens logic around fd_preXXX functions, ensuring they only
work on actual pre-opens. This also fixes the path returned in filestat
as we sometimes returned a full path, when typically the basename is the
only part that can be returned.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
This commit is contained in:
Crypt Keeper
2022-12-13 16:42:53 +09:00
committed by GitHub
parent 3b70504f3f
commit 2e13f57f56
7 changed files with 148 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
RATIONALE.md
View File

@@ -534,7 +534,8 @@ Compilers that target wasm act differently with regard to the working
directory. For example, while `GOOS=js` uses host functions to track the
working directory, WASI host functions do not. wasi-libc, used by TinyGo,
tracks working directory changes in compiled wasm instead: initially "/" until
code calls `chdir`.
code calls `chdir`. Zig assumes the first pre-opened file descriptor is the
working directory.
The only place wazero can standardize a layered concern is via a host function.
Since WASI doesn't use host functions to track the working directory, we can't
@@ -548,6 +549,7 @@ use absolute paths in configuration.
See
* https://github.com/golang/go/blob/go1.19rc2/src/syscall/fs_js.go#L324
* https://github.com/WebAssembly/wasi-libc/pull/214#issue-673090117
* https://github.com/ziglang/zig/blob/53a9ee699a35a3d245ab6d1dac1f0687a4dcb42c/src/main.zig#L32
### Why ignore the error returned by io.Reader when n > 1?
@@ -579,22 +581,43 @@ operation will err if it needs to. This helps reduce the complexity of the code
in wazero and also accommodates the scenario where the bytes read are enough to
satisfy its processor.
### FdPrestatDirName
### Pre-opened files
`FdPrestatDirName` is a WASI function to return the path of the pre-opened directory of a file descriptor.
It has the following three parameters, and the third `pathLen` has ambiguous semantics.
WASI includes `fd_prestat_get` and `fd_prestat_dir_name` functions used to
learn any directory paths for file descriptors open at initialization time.
- `fd` - a file descriptor
- `path` - the offset for the result path
- `pathLen` - In wazero, `FdPrestatDirName` writes the result path string to `path` offset for the exact length of `pathLen`.
For example, `__wasilibc_register_preopened_fd` scans any file descriptors past
STDERR (1) and invokes `fd_prestat_dir_name` to learn any path prefixes they
correspond to. Zig's `preopensAlloc` does similar. These pre-open functions are
not used again after initialization.
Wasmer considers `pathLen` to be the maximum length instead of the exact length that should be written.
wazero currently supports only one pre-opened file, "/" and so that is the name
returned by `fd_prestat_dir_name` for file descriptor 3 (STDERR+1).
See
* https://github.com/WebAssembly/wasi-libc/blob/a02298043ff551ce1157bc2ee7ab74c3bffe7144/libc-bottom-half/sources/preopens.c
* https://github.com/ziglang/zig/blob/9cb06f3b8bf9ea6b5e5307711bc97328762d6a1d/lib/std/fs/wasi.zig#L50-L53
### fd_prestat_dir_name
`fd_prestat_dir_name` is a WASI function to return the path of the pre-opened
directory of a file descriptor. It has the following three parameters, and the
third `path_len` has ambiguous semantics.
* `fd`: a file descriptor
* `path`: the offset for the result path
* `path_len`: In wazero, `FdPrestatDirName` writes the result path string to
`path` offset for the exact length of `path_len`.
Wasmer considers `path_len` to be the maximum length instead of the exact
length that should be written.
See https://github.com/wasmerio/wasmer/blob/3463c51268ed551933392a4063bd4f8e7498b0f6/lib/wasi/src/syscalls/mod.rs#L764
The semantics in wazero follows that of wasmtime.
See https://github.com/bytecodealliance/wasmtime/blob/2ca01ae9478f199337cf743a6ab543e8c3f3b238/crates/wasi-common/src/snapshots/preview_1.rs#L578-L582
Their semantics match when `pathLen` == the length of `path`, so in practice this difference won't matter match.
Their semantics match when `path_len` == the length of `path`, so in practice
this difference won't matter match.
## sys.Walltime and Nanotime

2
experimental/fs_test.go
View File

@@ -28,7 +28,7 @@ func TestWithFS(t *testing.T) {
entry, ok := fsCtx.OpenedFile(ctx, 3)
require.True(t, ok)
require.Equal(t, "/", entry.Path)
require.Equal(t, "/", entry.Name)
// Override to nil context, ex to block file access
ctx, closer, err = experimental.WithFS(ctx, nil)

20
imports/wasi_snapshot_preview1/fs.go
View File

@@ -462,6 +462,11 @@ func fdPrestatGetFn(ctx context.Context, mod api.Module, stack []uint64) (presta
sysCtx := mod.(*wasm.CallContext).Sys
fd := uint32(stack[0])
// Currently, we only pre-open the root file descriptor.
if fd != internalsys.FdRoot {
return 0, ErrnoBadf
}
entry, ok := sysCtx.FS(ctx).OpenedFile(ctx, fd)
if !ok {
return 0, ErrnoBadf
@@ -469,7 +474,7 @@ func fdPrestatGetFn(ctx context.Context, mod api.Module, stack []uint64) (presta
// Upper 32-bits are zero because...
// * Zero-value 8-bit tag, and 3-byte zero-value padding
prestat = uint64(len(entry.Path) << 32)
prestat = uint64(len(entry.Name) << 32)
errno = ErrnoSuccess
return
}
@@ -514,19 +519,22 @@ func fdPrestatDirNameFn(ctx context.Context, mod api.Module, params []uint64) Er
sysCtx := mod.(*wasm.CallContext).Sys
fd, path, pathLen := uint32(params[0]), uint32(params[1]), uint32(params[2])
// Currently, we only pre-open the root file descriptor.
if fd != internalsys.FdRoot {
return ErrnoBadf
}
f, ok := sysCtx.FS(ctx).OpenedFile(ctx, fd)
if !ok {
return ErrnoBadf
}
// Some runtimes may have another semantics. See /RATIONALE.md
if uint32(len(f.Path)) < pathLen {
if uint32(len(f.Name)) < pathLen {
return ErrnoNametoolong
}
// TODO: fdPrestatDirName may have to return ErrnoNotdir if the type of the
// prestat data of `fd` is not a PrestatDir.
if !mod.Memory().Write(ctx, path, []byte(f.Path)[:pathLen]) {
if !mod.Memory().Write(ctx, path, []byte(f.Name)[:pathLen]) {
return ErrnoFault
}
return ErrnoSuccess
@@ -1231,7 +1239,7 @@ func pathFilestatGetFn(ctx context.Context, mod api.Module, params []uint64) Err
return ErrnoNotdir // TODO: cache filetype instead of poking.
} else {
// TODO: consolidate "at" logic with path_open as same issues occur.
pathName = path.Join(dir.Path, pathName)
pathName = path.Join(dir.Name, pathName)
}
// Stat the file without allocating a file descriptor

60
imports/wasi_snapshot_preview1/fs_test.go
View File

@@ -729,9 +729,9 @@ func Test_fdPread_Errors(t *testing.T) {
}
func Test_fdPrestatGet(t *testing.T) {
pathName := "/tmp"
mod, fd, log, r := requireOpenFile(t, pathName, nil)
mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(fstest.MapFS{}))
defer r.Close(testCtx)
fd := internalsys.FdRoot // only pre-opened directory currently supported.
resultPrestat := uint32(1) // arbitrary offset
expectedMemory := []byte{
@@ -739,7 +739,7 @@ func Test_fdPrestatGet(t *testing.T) {
0, // 8-bit tag indicating `prestat_dir`, the only available tag
0, 0, 0, // 3-byte padding
// the result path length field after this
byte(len(pathName)), 0, 0, 0, // = in little endian encoding
1, 0, 0, 0, // = in little endian encoding
'?',
}
@@ -747,10 +747,10 @@ func Test_fdPrestatGet(t *testing.T) {
requireErrno(t, ErrnoSuccess, mod, fdPrestatGetName, uint64(fd), uint64(resultPrestat))
require.Equal(t, `
--> proxy.fd_prestat_get(fd=4,result.prestat=1)
--> wasi_snapshot_preview1.fd_prestat_get(fd=4,result.prestat=1)
==> wasi_snapshot_preview1.fdPrestatGet(fd=4)
<== (prestat=17179869184,ESUCCESS)
--> proxy.fd_prestat_get(fd=3,result.prestat=1)
--> wasi_snapshot_preview1.fd_prestat_get(fd=3,result.prestat=1)
==> wasi_snapshot_preview1.fdPrestatGet(fd=3)
<== (prestat=4294967296,ESUCCESS)
<-- ESUCCESS
<-- 0
`, "\n"+log.String())
@@ -761,9 +761,9 @@ func Test_fdPrestatGet(t *testing.T) {
}
func Test_fdPrestatGet_Errors(t *testing.T) {
pathName := "/tmp"
mod, fd, log, r := requireOpenFile(t, pathName, nil)
mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(fstest.MapFS{}))
defer r.Close(testCtx)
fd := internalsys.FdRoot // only pre-opened directory currently supported.
memorySize := mod.Memory().Size(testCtx)
tests := []struct {
@@ -793,8 +793,8 @@ func Test_fdPrestatGet_Errors(t *testing.T) {
resultPrestat: memorySize,
expectedErrno: ErrnoFault,
expectedLog: `
--> proxy.fd_prestat_get(fd=4,result.prestat=65536)
--> wasi_snapshot_preview1.fd_prestat_get(fd=4,result.prestat=65536)
--> proxy.fd_prestat_get(fd=3,result.prestat=65536)
--> wasi_snapshot_preview1.fd_prestat_get(fd=3,result.prestat=65536)
<-- EFAULT
<-- 21
`,
@@ -815,24 +815,22 @@ func Test_fdPrestatGet_Errors(t *testing.T) {
}
func Test_fdPrestatDirName(t *testing.T) {
pathName := "/tmp"
mod, fd, log, r := requireOpenFile(t, pathName, nil)
mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(fstest.MapFS{}))
defer r.Close(testCtx)
fd := internalsys.FdRoot // only pre-opened directory currently supported.
path := uint32(1) // arbitrary offset
pathLen := uint32(3) // shorter than len("/tmp") to test the path is written for the length of pathLen
pathLen := uint32(0) // shorter than len("/") to prove truncation is ok
expectedMemory := []byte{
'?',
'/', 't', 'm',
'?', '?', '?',
'?', '?', '?', '?',
}
maskMemory(t, testCtx, mod, len(expectedMemory))
requireErrno(t, ErrnoSuccess, mod, fdPrestatDirNameName, uint64(fd), uint64(path), uint64(pathLen))
require.Equal(t, `
--> proxy.fd_prestat_dir_name(fd=4,path=1,path_len=3)
==> wasi_snapshot_preview1.fd_prestat_dir_name(fd=4,path=1,path_len=3)
--> proxy.fd_prestat_dir_name(fd=3,path=1,path_len=0)
==> wasi_snapshot_preview1.fd_prestat_dir_name(fd=3,path=1,path_len=0)
<== ESUCCESS
<-- 0
`, "\n"+log.String())
@@ -843,13 +841,13 @@ func Test_fdPrestatDirName(t *testing.T) {
}
func Test_fdPrestatDirName_Errors(t *testing.T) {
pathName := "/tmp"
mod, fd, log, r := requireOpenFile(t, pathName, nil)
mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(fstest.MapFS{}))
defer r.Close(testCtx)
fd := internalsys.FdRoot // only pre-opened directory currently supported.
memorySize := mod.Memory().Size(testCtx)
validAddress := uint32(0) // Arbitrary valid address as arguments to fd_prestat_dir_name. We chose 0 here.
pathLen := uint32(len("/tmp"))
pathLen := uint32(len("/"))
tests := []struct {
name string
@@ -866,8 +864,8 @@ func Test_fdPrestatDirName_Errors(t *testing.T) {
pathLen: pathLen,
expectedErrno: ErrnoFault,
expectedLog: `
--> proxy.fd_prestat_dir_name(fd=4,path=65536,path_len=4)
==> wasi_snapshot_preview1.fd_prestat_dir_name(fd=4,path=65536,path_len=4)
--> proxy.fd_prestat_dir_name(fd=3,path=65536,path_len=1)
==> wasi_snapshot_preview1.fd_prestat_dir_name(fd=3,path=65536,path_len=1)
<== EFAULT
<-- 21
`,
@@ -879,8 +877,8 @@ func Test_fdPrestatDirName_Errors(t *testing.T) {
pathLen: pathLen,
expectedErrno: ErrnoFault,
expectedLog: `
--> proxy.fd_prestat_dir_name(fd=4,path=65533,path_len=4)
==> wasi_snapshot_preview1.fd_prestat_dir_name(fd=4,path=65533,path_len=4)
--> proxy.fd_prestat_dir_name(fd=3,path=65536,path_len=1)
==> wasi_snapshot_preview1.fd_prestat_dir_name(fd=3,path=65536,path_len=1)
<== EFAULT
<-- 21
`,
@@ -892,8 +890,8 @@ func Test_fdPrestatDirName_Errors(t *testing.T) {
pathLen: pathLen + 1,
expectedErrno: ErrnoNametoolong,
expectedLog: `
--> proxy.fd_prestat_dir_name(fd=4,path=0,path_len=5)
==> wasi_snapshot_preview1.fd_prestat_dir_name(fd=4,path=0,path_len=5)
--> proxy.fd_prestat_dir_name(fd=3,path=0,path_len=2)
==> wasi_snapshot_preview1.fd_prestat_dir_name(fd=3,path=0,path_len=2)
<== ENAMETOOLONG
<-- 37
`,
@@ -905,8 +903,8 @@ func Test_fdPrestatDirName_Errors(t *testing.T) {
pathLen: pathLen,
expectedErrno: ErrnoBadf,
expectedLog: `
--> proxy.fd_prestat_dir_name(fd=42,path=0,path_len=4)
==> wasi_snapshot_preview1.fd_prestat_dir_name(fd=42,path=0,path_len=4)
--> proxy.fd_prestat_dir_name(fd=42,path=0,path_len=1)
==> wasi_snapshot_preview1.fd_prestat_dir_name(fd=42,path=0,path_len=1)
<== EBADF
<-- 8
`,
@@ -2654,7 +2652,7 @@ func Test_pathOpen(t *testing.T) {
fsc := mod.(*wasm.CallContext).Sys.FS(testCtx)
f, ok := fsc.OpenedFile(testCtx, expectedFD)
require.True(t, ok)
require.Equal(t, pathName, f.Path)
require.Equal(t, pathName, f.Name)
}
func Test_pathOpen_Errors(t *testing.T) {

14
internal/sys/fs.go
View File

@@ -75,11 +75,13 @@ func (emptyRootDir) Sys() interface{} { return nil }
// FileEntry maps a path to an open file in a file system.
type FileEntry struct {
// Path was the argument to FSContext.OpenFile
// TODO: we may need an additional field which is the full path.
Path string
// Name is the basename of the file, at the time it was opened. When the
// file is root "/" (fd = FdRoot), this is "/".
//
// Note: This must match fs.FileInfo.
Name string
// File is always non-nil, even when root "/" (fd=FdRoot)
// File is always non-nil, even when root "/" (fd = FdRoot).
File fs.File
// ReadDir is present when this File is a fs.ReadDirFile and `ReadDir`
@@ -159,7 +161,7 @@ func NewFSContext(root fs.FS) (fsc *FSContext, err error) {
return &FSContext{
fs: root,
openedFiles: map[uint32]*FileEntry{
FdRoot: {Path: "/", File: rootDir},
FdRoot: {Name: "/", File: rootDir},
},
lastFD: FdRoot,
}, nil
@@ -197,7 +199,7 @@ func (c *FSContext) OpenFile(_ context.Context, name string /* TODO: flags int,
_ = f.Close()
return 0, syscall.EBADF
}
c.openedFiles[newFD] = &FileEntry{Path: name, File: f}
c.openedFiles[newFD] = &FileEntry{Name: path.Base(name), File: f}
return newFD, nil
}

76
internal/sys/fs_test.go
View File

@@ -4,8 +4,10 @@ import (
"context"
"embed"
"errors"
"io"
"io/fs"
"os"
"path"
"testing"
"testing/fstest"
@@ -19,13 +21,6 @@ var testCtx = context.WithValue(context.Background(), struct{}{}, "arbitrary")
//go:embed testdata
var testdata embed.FS
var testFS = fstest.MapFS{
"empty.txt": {},
"test.txt": {Data: []byte("animals\n")},
"sub": {Mode: fs.ModeDir},
"sub/test.txt": {Data: []byte("greet sub dir\n")},
}
func TestNewFSContext(t *testing.T) {
embedFS, err := fs.Sub(testdata, "testdata")
require.NoError(t, err)
@@ -49,11 +44,7 @@ func TestNewFSContext(t *testing.T) {
},
{
name: "fstest.MapFS",
fs: testFS,
},
{
name: "fstest.MapFS",
fs: testFS,
fs: fstest.MapFS{},
},
}
@@ -66,7 +57,7 @@ func TestNewFSContext(t *testing.T) {
defer fsc.Close(testCtx)
require.Equal(t, tc.fs, fsc.fs)
require.Equal(t, "/", fsc.openedFiles[FdRoot].Path)
require.Equal(t, "/", fsc.openedFiles[FdRoot].Name)
rootFile := fsc.openedFiles[FdRoot].File
require.NotNil(t, rootFile)
@@ -118,6 +109,65 @@ func TestEmptyFSContext(t *testing.T) {
})
}
func TestContext_File(t *testing.T) {
embedFS, err := fs.Sub(testdata, "testdata")
require.NoError(t, err)
fsc, err := NewFSContext(embedFS)
require.NoError(t, err)
defer fsc.Close(testCtx)
tests := []struct {
name string
expected string
}{
{
name: "empty.txt",
},
{
name: "test.txt",
expected: "animals\n",
},
{
name: "sub/test.txt",
expected: "greet sub dir\n",
},
{
name: "sub/sub/test.txt",
expected: "greet sub sub dir\n",
},
}
for _, tt := range tests {
tc := tt
t.Run(tc.name, func(b *testing.T) {
fd, err := fsc.OpenFile(testCtx, tc.name)
require.NoError(t, err)
defer fsc.CloseFile(testCtx, fd)
f, ok := fsc.OpenedFile(testCtx, fd)
require.True(t, ok)
stat, err := f.File.Stat()
require.NoError(t, err)
// Ensure the name is the basename and matches the stat name.
require.Equal(t, path.Base(tc.name), f.Name)
require.Equal(t, f.Name, stat.Name())
buf := make([]byte, stat.Size())
size, err := f.File.Read(buf)
if err != nil {
require.Equal(t, io.EOF, err)
}
require.Equal(t, stat.Size(), int64(size))
require.Equal(t, tc.expected, string(buf[:size]))
})
}
}
func TestContext_Close(t *testing.T) {
fsc, err := NewFSContext(testfs.FS{"foo": &testfs.File{}})
require.NoError(t, err)

1
internal/sys/testdata/sub/sub/test.txt vendored Normal file
View File

@@ -0,0 +1 @@
greet sub sub dir