Repository: tetratelabs/wazero
Pull request #1256 (2023-03-20 01:05:41)

interpret RIGHT_FD_WRITE to set O_RDWR on files opened by wazero
Following up from our conversation on Slack, this PR demonstrate a fix which can be used to allow client programs to open files in write mode when they provided none of the O_CREATE, O_APPEND, or O_TRUNC flags.



Patch:
diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go
index 43fe3af7..615fcc64 100644
--- a/imports/wasi_snapshot_preview1/fs.go
+++ b/imports/wasi_snapshot_preview1/fs.go
@@ -1548,7 +1548,7 @@ func pathLinkFn(_ context.Context, mod api.Module, params []uint64) Errno {
 //   - path: offset in api.Memory to read the path string from
 //   - pathLen: length of `path`
 //   - oFlags: open flags to indicate the method by which to open the file
-//   - fsRightsBase: ignored as rights were removed from WASI.
+//   - fsRightsBase: interpret RIGHT_FD_WRITE to set O_RDWR
 //   - fsRightsInheriting: ignored as rights were removed from WASI.
 //     created file descriptor for `path`
 //   - fdFlags: file descriptor flags
@@ -1611,8 +1611,9 @@ func pathOpenFn(_ context.Context, mod api.Module, params []uint64) Errno {
 
 	oflags := uint16(params[4])
 
-	// rights aren't used
-	_, _ = params[5], params[6]
+	rights := uint32(params[5])
+	// inherited rights aren't used
+	_ = params[6]
 
 	fdflags := uint16(params[7])
 	resultOpenedFd := uint32(params[8])
@@ -1622,7 +1623,7 @@ func pathOpenFn(_ context.Context, mod api.Module, params []uint64) Errno {
 		return errno
 	}
 
-	fileOpenFlags := openFlags(dirflags, oflags, fdflags)
+	fileOpenFlags := openFlags(dirflags, oflags, fdflags, rights)
 	isDir := fileOpenFlags&platform.O_DIRECTORY != 0
 
 	if isDir && oflags&O_CREAT != 0 {
@@ -1701,7 +1702,7 @@ func preopenPath(fsc *sys.FSContext, fd uint32) (string, Errno) {
 	}
 }
 
-func openFlags(dirflags, oflags, fdflags uint16) (openFlags int) {
+func openFlags(dirflags, oflags, fdflags uint16, rights uint32) (openFlags int) {
 	if dirflags&LOOKUP_SYMLINK_FOLLOW == 0 {
 		openFlags |= platform.O_NOFOLLOW
 	}
@@ -1720,6 +1721,13 @@ func openFlags(dirflags, oflags, fdflags uint16) (openFlags int) {
 	if fdflags&FD_APPEND != 0 {
 		openFlags |= syscall.O_RDWR | syscall.O_APPEND
 	}
+	// Since rights were discontinued in wasi, we only interpret RIGHT_FD_WRITE
+	// because it is the only way to know that we need to set write permissions
+	// on a file if the application did not pass any of O_CREATE, O_APPEND, nor
+	// O_TRUNC.
+	if rights&RIGHT_FD_WRITE != 0 {
+		openFlags |= syscall.O_RDWR
+	}
 	if openFlags == 0 {
 		openFlags = syscall.O_RDONLY
 	}
diff --git a/internal/sys/fs.go b/internal/sys/fs.go
index 46feff9a..21e8a642 100644
--- a/internal/sys/fs.go
+++ b/internal/sys/fs.go
@@ -194,7 +194,6 @@ func (f *FileEntry) CachedStat() (ino uint64, fileType fs.FileMode, err error) {
 		if err = f.Stat(&st); err != nil {
 			return
 		}
-		f.cachedStat = &cachedStat{Ino: st.Ino, Type: st.Mode & fs.ModeType}
 	}
 	return f.cachedStat.Ino, f.cachedStat.Type, nil
 }
@@ -211,7 +210,7 @@ func (f *FileEntry) Stat(st *platform.Stat_t) (err error) {
 	}
 
 	if err == nil {
-		f.cachedStat = &cachedStat{Ino: st.Ino, Type: st.Mode}
+		f.cachedStat = &cachedStat{Ino: st.Ino, Type: st.Mode & fs.ModeType}
 	}
 	return
 }


Test patch:
diff --git a/imports/wasi_snapshot_preview1/fs_test.go b/imports/wasi_snapshot_preview1/fs_test.go
index f320dc7f..d8cfb2c2 100644
--- a/imports/wasi_snapshot_preview1/fs_test.go
+++ b/imports/wasi_snapshot_preview1/fs_test.go
@@ -3711,6 +3711,7 @@ func Test_pathOpen(t *testing.T) {
 		path          func(t *testing.T) string
 		oflags        uint16
 		fdflags       uint16
+		rights        uint32
 		expected      func(t *testing.T, fsc *sys.FSContext)
 		expectedErrno Errno
 		expectedLog   string
@@ -3897,6 +3898,20 @@ func Test_pathOpen(t *testing.T) {
 			expectedLog: `
 ==> wasi_snapshot_preview1.path_open(fd=3,dirflags=,path=trunc,oflags=TRUNC,fs_rights_base=,fs_rights_inheriting=,fdflags=)
 <== (opened_fd=4,errno=ESUCCESS)
+`,
+		},
+		{
+			name:   "sysfs.DirFS RIGHT_FD_WRITE",
+			fs:     writeFS,
+			path:   func(*testing.T) string { return fileName },
+			oflags: 0,
+			rights: RIGHT_FD_WRITE,
+			expected: func(t *testing.T, fsc *sys.FSContext) {
+				requireContents(t, fsc, expectedOpenedFd, fileName, fileContents)
+			},
+			expectedLog: `
+==> wasi_snapshot_preview1.path_open(fd=3,dirflags=,path=file,oflags=,fs_rights_base=FD_WRITE,fs_rights_inheriting=,fdflags=)
+<== (opened_fd=4,errno=ESUCCESS)
 `,
 		},
 	}
@@ -3920,11 +3935,11 @@ func Test_pathOpen(t *testing.T) {
 			// https://github.com/WebAssembly/WASI/blob/snapshot-01/phases/snapshot/docs.md#lookupflags
 			dirflags := 0
 
-			// rights aren't used
-			fsRightsBase, fsRightsInheriting := uint64(0), uint64(0)
+			// inherited rights aren't used
+			fsRightsInheriting := uint64(0)
 
 			requireErrnoResult(t, tc.expectedErrno, mod, PathOpenName, uint64(fd), uint64(dirflags), uint64(path),
-				uint64(pathLen), uint64(tc.oflags), fsRightsBase, fsRightsInheriting, uint64(tc.fdflags), uint64(resultOpenedFd))
+				uint64(pathLen), uint64(tc.oflags), uint64(tc.rights), fsRightsInheriting, uint64(tc.fdflags), uint64(resultOpenedFd))
 			require.Equal(t, tc.expectedLog, "\n"+log.String())
 
 			if tc.expectedErrno == ErrnoSuccess {
diff --git a/imports/wasi_snapshot_preview1/fs_unit_test.go b/imports/wasi_snapshot_preview1/fs_unit_test.go
index ab7bf51d..dcb01fa0 100644
--- a/imports/wasi_snapshot_preview1/fs_unit_test.go
+++ b/imports/wasi_snapshot_preview1/fs_unit_test.go
@@ -371,6 +371,7 @@ func Test_openFlags(t *testing.T) {
 	tests := []struct {
 		name                      string
 		dirflags, oflags, fdflags uint16
+		rights                    uint32
 		expectedOpenFlags         int
 	}{
 		{
@@ -412,13 +413,18 @@ func Test_openFlags(t *testing.T) {
 			dirflags:          LOOKUP_SYMLINK_FOLLOW,
 			expectedOpenFlags: syscall.O_RDONLY,
 		},
+		{
+			name:              "rights=FD_WRITE",
+			rights:            RIGHT_FD_WRITE,
+			expectedOpenFlags: platform.O_NOFOLLOW | syscall.O_RDWR,
+		},
 	}
 
 	for _, tt := range tests {
 		tc := tt
 
 		t.Run(tc.name, func(t *testing.T) {
-			openFlags := openFlags(tc.dirflags, tc.oflags, tc.fdflags)
+			openFlags := openFlags(tc.dirflags, tc.oflags, tc.fdflags, tc.rights)
 			require.Equal(t, tc.expectedOpenFlags, openFlags)
 		})
 	}