Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix path_symlink only works when dir mounted to / #2229

Merged
merged 4 commits into from
Jun 6, 2024
Merged

Fix path_symlink only works when dir mounted to / #2229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e8800b5
Fix path_symlink only works when dir mounted to /
yagehu Jun 5, 2024
31b2fd4
Add test case for path_symlink fix
yagehu Jun 5, 2024
0b0ad7e
Fix casing
yagehu Jun 5, 2024
83770c3
Fix path_symlink test name
yagehu Jun 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions imports/wasi_snapshot_preview1/fs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1951,16 +1951,16 @@ func pathSymlinkFn(_ context.Context, mod api.Module, params []uint64) experimen
return experimentalsys.EFAULT
}

newPathBuf, ok := mem.Read(newPath, newPathLen)
if !ok {
return experimentalsys.EFAULT
_, newPathName, errno := atPath(fsc, mod.Memory(), fd, newPath, newPathLen)
if errno != 0 {
return errno
}

return dir.FS.Symlink(
// Do not join old path since it's only resolved when dereference the link created here.
// And the dereference result depends on the opening directory's file descriptor at that point.
bufToStr(oldPathBuf),
path.Join(dir.Name, bufToStr(newPathBuf)),
newPathName,
)
}

Expand Down
52 changes: 42 additions & 10 deletions imports/wasi_snapshot_preview1/fs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4497,7 +4497,7 @@ func Test_pathRemoveDirectory_Errors(t *testing.T) {
}
}

func Test_pathSymlink_errors(t *testing.T) {
func Test_pathSymlink(t *testing.T) {
tmpDir := t.TempDir() // open before loop to ensure no locking problems.

dirName := "dir"
Expand Down Expand Up @@ -4525,14 +4525,46 @@ func Test_pathSymlink_errors(t *testing.T) {
ok = mem.Write(link, []byte(linkName))
require.True(t, ok)

t.Run("success", func(t *testing.T) {
requireErrnoResult(t, wasip1.ErrnoSuccess, mod, wasip1.PathSymlinkName,
uint64(file), uint64(len(fileName)), uint64(fd), uint64(link), uint64(len(linkName)))
require.Contains(t, log.String(), wasip1.ErrnoName(wasip1.ErrnoSuccess))
st, err := os.Lstat(joinPath(dirPath, linkName))
require.NoError(t, err)
require.Equal(t, st.Mode()&os.ModeSymlink, os.ModeSymlink)
})
successCases := []struct {
mathetake marked this conversation as resolved.
Show resolved Hide resolved
name string
fd int32
oldPath string
newPath string
}{
{
name: "dir",
fd: fd,
oldPath: fileName,
newPath: linkName,
},
{
name: "preopened root",
fd: sys.FdPreopen,
oldPath: fileName,
newPath: linkName,
},
}

for _, tt := range successCases {
tc := tt

t.Run(tc.name, func(t *testing.T) {
file := uint32(0xbb)
ok := mem.Write(file, []byte(fileName))
require.True(t, ok)

link := uint32(0xdd)
ok = mem.Write(link, []byte(linkName))
require.True(t, ok)

requireErrnoResult(t, wasip1.ErrnoSuccess, mod, wasip1.PathSymlinkName,
uint64(file), uint64(len(tc.oldPath)), uint64(tc.fd), uint64(link), uint64(len(tc.newPath)))
require.Contains(t, log.String(), wasip1.ErrnoName(wasip1.ErrnoSuccess))
st, err := os.Lstat(joinPath(dirPath, linkName))
require.NoError(t, err)
require.Equal(t, st.Mode()&os.ModeSymlink, os.ModeSymlink)
})
}

t.Run("errors", func(t *testing.T) {
for _, tc := range []struct {
Expand Down Expand Up @@ -4941,7 +4973,7 @@ func requireOpenFile(t *testing.T, tmpDir string, pathName string, data []byte,
if readOnly {
fsConfig = fsConfig.WithReadOnlyDirMount(tmpDir, "/")
} else {
fsConfig = fsConfig.WithDirMount(tmpDir, "/")
fsConfig = fsConfig.WithDirMount(tmpDir, "preopen")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is necessary to expose the problem because the original path join would discard the /, masking the bug. Preopening a directory and mounting it at a non-root location would cause the bug.

}

mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFSConfig(fsConfig))
Expand Down
Loading