Repository: tetratelabs/wazero
Pull request #1648 (2023-08-22 19:51:16)

wasi: fix symlink-related issues with path handling
- Fixes a bug in handling the buffer when the write goes over the given limit. ERANGE should be returned.
- Removes dependency from the syscall package in `fs.go`, `experimentalsys.O_WRONLY` should be used instead (portability)

evacchi: 
- `symlink_filestat`

      ==> wasi_snapshot_preview1.path_filestat_get(fd=5,flags=,path=symlink)
      <== (filestat={filetype=SYMBOLIC_LINK,size=4,mtim=1692707577959674593},errno=ESUCCESS)
      thread 'main' panicked at 'assertion failed: `(left == right)`
        left: `1692707577959674593`,
       right: `1692707577959674393`: symlink mtim should change', src/bin/symlink_filestat.rs:46:5


          // Check that symlink mtim motification worked
          let modified_sym_stat = wasi::path_filestat_get(dir_fd, 0, "symlink")
              .expect("reading file stats after path_filestat_set_times");
          assert_eq!(
              modified_sym_stat.mtim, sym_new_mtim,
              "symlink mtim should change"
          );


I noticed that [`utimens` actually supports a NOFOLLOW flag](https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html#:~:text=fcntl.h%3E%3A-,AT_SYMLINK_NOFOLLOW,-If%20path%20names), which might allow us to avoid reopening the file (which I don't think is working as expected anyway; it looks like it is always dereferencing the link:

https://github.com/tetratelabs/wazero/blob/967d8df56d018386a821a3c340d4f085aa9750bd/imports/wasi_snapshot_preview1/fs.go#L1461-L1471

also notice that we are using syscall.O_WRONLY while I think it should be experimentalists.O_WRONLY. if I pass O_NOFOLLOW however I get back an ELOOP (weird?)

....however [adding a flag to Utimens will require us to break the API of the experimental VFS (`type FS` in `sys/fs.go`)](https://github.com/tetratelabs/wazero/blob/967d8df56d018386a821a3c340d4f085aa9750bd/experimental/sys/fs.go#L292). I think it may be acceptable since it is experimental, but breaking a public API is never nice. I am open to suggestions.
evacchi: repurposing this PR to only fix the actual issues, dropping the edge cases for path handling in symlinks for now.

Patch:
diff --git a/experimental/sys/errno.go b/experimental/sys/errno.go
index 81baadd0..23894949 100644
--- a/experimental/sys/errno.go
+++ b/experimental/sys/errno.go
@@ -35,6 +35,7 @@ const (
 	ENOENT
 	ENOSYS
 	ENOTDIR
+	ERANGE
 	ENOTEMPTY
 	ENOTSOCK
 	ENOTSUP
@@ -79,6 +80,8 @@ func (e Errno) Error() string {
 		return "functionality not supported"
 	case ENOTDIR:
 		return "not a directory or a symbolic link to a directory"
+	case ERANGE:
+		return "result too large"
 	case ENOTEMPTY:
 		return "directory not empty"
 	case ENOTSOCK:
diff --git a/experimental/sys/syscall_errno.go b/experimental/sys/syscall_errno.go
index 23c4be92..d28f6faa 100644
--- a/experimental/sys/syscall_errno.go
+++ b/experimental/sys/syscall_errno.go
@@ -40,6 +40,8 @@ func syscallToErrno(err error) (Errno, bool) {
 		return ENOSYS, true
 	case syscall.ENOTDIR:
 		return ENOTDIR, true
+	case syscall.ERANGE:
+		return ERANGE, true
 	case syscall.ENOTEMPTY:
 		return ENOTEMPTY, true
 	case syscall.ENOTSOCK:
diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go
index b80c14d7..9d28ae17 100644
--- a/imports/wasi_snapshot_preview1/fs.go
+++ b/imports/wasi_snapshot_preview1/fs.go
@@ -7,7 +7,6 @@ import (
 	"math"
 	"path"
 	"strings"
-	"syscall"
 	"unsafe"
 
 	"github.com/tetratelabs/wazero/api"
@@ -1463,7 +1462,7 @@ func pathFilestatSetTimesFn(_ context.Context, mod api.Module, params []uint64)
 		return preopen.Utimens(pathName, atim, mtim)
 	}
 	// Otherwise, we need to emulate don't follow by opening the file by path.
-	if f, errno := preopen.OpenFile(pathName, syscall.O_WRONLY, 0); errno != 0 {
+	if f, errno := preopen.OpenFile(pathName, experimentalsys.O_WRONLY, 0); errno != 0 {
 		return errno
 	} else {
 		defer f.Close()
@@ -1797,6 +1796,10 @@ func pathReadlinkFn(_ context.Context, mod api.Module, params []uint64) experime
 		return errno
 	}
 
+	if len(dst) > int(bufLen) {
+		return experimentalsys.ERANGE
+	}
+
 	if ok := mem.WriteString(buf, dst); !ok {
 		return experimentalsys.EFAULT
 	}
diff --git a/internal/wasip1/errno.go b/internal/wasip1/errno.go
index b4af4150..701c2aa4 100644
--- a/internal/wasip1/errno.go
+++ b/internal/wasip1/errno.go
@@ -296,6 +296,8 @@ func ToErrno(errno sys.Errno) Errno {
 		return ErrnoNosys
 	case sys.ENOTDIR:
 		return ErrnoNotdir
+	case sys.ERANGE:
+		return ErrnoRange
 	case sys.ENOTEMPTY:
 		return ErrnoNotempty
 	case sys.ENOTSOCK:


Test patch:
diff --git a/imports/wasi_snapshot_preview1/fs_test.go b/imports/wasi_snapshot_preview1/fs_test.go
index a6219550..fc23e897 100644
--- a/imports/wasi_snapshot_preview1/fs_test.go
+++ b/imports/wasi_snapshot_preview1/fs_test.go
@@ -4238,12 +4238,12 @@ func Test_pathReadlink(t *testing.T) {
 			{expectedErrno: wasip1.ErrnoInval, bufLen: 100},
 			{
 				name:          "bufLen too short",
-				expectedErrno: wasip1.ErrnoFault,
+				expectedErrno: wasip1.ErrnoRange,
 				fd:            dirFD,
 				bufLen:        10,
 				path:          destinationPath,
 				pathLen:       uint32(len(destinationPathName)),
-				buf:           math.MaxUint32,
+				buf:           0,
 			},
 			{
 				name:          "path past memory",