From 4b66502ddaa264257343aae58395ef8cd4176cfd Mon Sep 17 00:00:00 2001 From: Achille Roussel Date: Sat, 29 Apr 2023 11:17:25 -0700 Subject: [PATCH] syscall: fix opening of directories on wasip1 Go programs targeting GOOS=wasip1 were failing to open directories when executed with runtimes like wasmtime or wasmedge due to requesting rights for operations that are not supported on directories such as fd_read, fd_write, etc... This change addresses the issue by performing a second path_open when observing EISDIR, and masking the requested rights to only ask for permissions to perform operations supported by a directory. Change-Id: Ibf65acf4a38bc848a649f41dbd026507d8b63c82 Reviewed-on: https://go-review.googlesource.com/c/go/+/490755 Auto-Submit: Ian Lance Taylor Run-TryBot: Ian Lance Taylor Reviewed-by: Dmitri Shuralyov Reviewed-by: Ian Lance Taylor TryBot-Result: Gopher Robot --- src/syscall/fs_wasip1.go | 87 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 83 insertions(+), 4 deletions(-) diff --git a/src/syscall/fs_wasip1.go b/src/syscall/fs_wasip1.go index ef04af69660..ab00e5ba22e 100644 --- a/src/syscall/fs_wasip1.go +++ b/src/syscall/fs_wasip1.go @@ -100,6 +100,63 @@ const ( fullRights = rights(^uint32(0)) readRights = rights(RIGHT_FD_READ | RIGHT_FD_READDIR) writeRights = rights(RIGHT_FD_DATASYNC | RIGHT_FD_WRITE | RIGHT_FD_ALLOCATE | RIGHT_PATH_FILESTAT_SET_SIZE) + + // Some runtimes have very strict expectations when it comes to which + // rights can be enabled on files opened by path_open. The fileRights + // constant is used as a mask to retain only bits for operations that + // are supported on files. + fileRights rights = RIGHT_FD_DATASYNC | + RIGHT_FD_READ | + RIGHT_FD_SEEK | + RIGHT_FDSTAT_SET_FLAGS | + RIGHT_FD_SYNC | + RIGHT_FD_TELL | + RIGHT_FD_WRITE | + RIGHT_FD_ADVISE | + RIGHT_FD_ALLOCATE | + RIGHT_PATH_CREATE_DIRECTORY | + RIGHT_PATH_CREATE_FILE | + RIGHT_PATH_LINK_SOURCE | + RIGHT_PATH_LINK_TARGET | + RIGHT_PATH_OPEN | + RIGHT_FD_READDIR | + RIGHT_PATH_READLINK | + RIGHT_PATH_RENAME_SOURCE | + RIGHT_PATH_RENAME_TARGET | + RIGHT_PATH_FILESTAT_GET | + RIGHT_PATH_FILESTAT_SET_SIZE | + RIGHT_PATH_FILESTAT_SET_TIMES | + RIGHT_FD_FILESTAT_GET | + RIGHT_FD_FILESTAT_SET_SIZE | + RIGHT_FD_FILESTAT_SET_TIMES | + RIGHT_PATH_SYMLINK | + RIGHT_PATH_REMOVE_DIRECTORY | + RIGHT_PATH_UNLINK_FILE | + RIGHT_POLL_FD_READWRITE + + // Runtimes like wasmtime and wasmedge will refuse to open directories + // if the rights requested by the application exceed the operations that + // can be performed on a directory. + dirRights rights = RIGHT_FD_SEEK | + RIGHT_FDSTAT_SET_FLAGS | + RIGHT_FD_SYNC | + RIGHT_PATH_CREATE_DIRECTORY | + RIGHT_PATH_CREATE_FILE | + RIGHT_PATH_LINK_SOURCE | + RIGHT_PATH_LINK_TARGET | + RIGHT_PATH_OPEN | + RIGHT_FD_READDIR | + RIGHT_PATH_READLINK | + RIGHT_PATH_RENAME_SOURCE | + RIGHT_PATH_RENAME_TARGET | + RIGHT_PATH_FILESTAT_GET | + RIGHT_PATH_FILESTAT_SET_SIZE | + RIGHT_PATH_FILESTAT_SET_TIMES | + RIGHT_FD_FILESTAT_GET | + RIGHT_FD_FILESTAT_SET_TIMES | + RIGHT_PATH_SYMLINK | + RIGHT_PATH_REMOVE_DIRECTORY | + RIGHT_PATH_UNLINK_FILE ) // https://github.com/WebAssembly/WASI/blob/a2b96e81c0586125cc4dc79a5be0b78d9a059925/legacy/preview1/docs.md#-fd_closefd-fd---result-errno @@ -435,11 +492,11 @@ func Open(path string, openmode int, perm uint32) (int, error) { var rights rights switch openmode & (O_RDONLY | O_WRONLY | O_RDWR) { case O_RDONLY: - rights = fullRights & ^writeRights + rights = fileRights & ^writeRights case O_WRONLY: - rights = fullRights & ^readRights + rights = fileRights & ^readRights case O_RDWR: - rights = fullRights + rights = fileRights } var fdflags fdflags @@ -458,10 +515,32 @@ func Open(path string, openmode int, perm uint32) (int, error) { pathLen, oflags, rights, - fullRights, + fileRights, fdflags, unsafe.Pointer(&fd), ) + if errno == EISDIR && oflags == 0 && fdflags == 0 && ((rights & writeRights) == 0) { + // wasmtime and wasmedge will error if attempting to open a directory + // because we are asking for too many rights. However, we cannot + // determine ahread of time if the path we are about to open is a + // directory, so instead we fallback to a second call to path_open with + // a more limited set of rights. + // + // This approach is subject to a race if the file system is modified + // concurrently, so we also inject OFLAG_DIRECTORY to ensure that we do + // not accidentally open a file which is not a directory. + errno = path_open( + dirFd, + LOOKUP_SYMLINK_FOLLOW, + pathPtr, + pathLen, + oflags|OFLAG_DIRECTORY, + rights&dirRights, + fileRights, + fdflags, + unsafe.Pointer(&fd), + ) + } return int(fd), errnoErr(errno) }