From 17ff6063a37551e83eebab98616a21bbc7e58764 Mon Sep 17 00:00:00 2001 From: Wei Zhang Date: Mon, 22 Apr 2019 20:06:09 -0700 Subject: [PATCH] Bugfix: fix fstatat symbol link to dir For a symbol link to some directory, eg. `/tmp/symlink -> /tmp/dir` `fstatat("/tmp/symlink")` should return symbol link data, but `fstatat("/tmp/symlink/")` (symlink with trailing slash) should return directory data it points following linux behaviour. Currently fstatat() a symlink with trailing slash will get "not a directory" error which is wrong. Signed-off-by: Wei Zhang Change-Id: I63469b1fb89d083d1c1255d32d52864606fbd7e2 PiperOrigin-RevId: 244783916 --- pkg/sentry/syscalls/linux/sys_stat.go | 6 +- test/syscalls/linux/stat.cc | 92 +++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/pkg/sentry/syscalls/linux/sys_stat.go b/pkg/sentry/syscalls/linux/sys_stat.go index 02634b2dd..49c225011 100644 --- a/pkg/sentry/syscalls/linux/sys_stat.go +++ b/pkg/sentry/syscalls/linux/sys_stat.go @@ -63,7 +63,11 @@ func Fstatat(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Sysca return 0, nil, fstat(t, file, statAddr) } - return 0, nil, fileOpOn(t, fd, path, flags&linux.AT_SYMLINK_NOFOLLOW == 0, func(root *fs.Dirent, d *fs.Dirent) error { + // If the path ends in a slash (i.e. dirPath is true) or if AT_SYMLINK_NOFOLLOW is unset, + // then we must resolve the final component. + resolve := dirPath || flags&linux.AT_SYMLINK_NOFOLLOW == 0 + + return 0, nil, fileOpOn(t, fd, path, resolve, func(root *fs.Dirent, d *fs.Dirent) error { return stat(t, d, dirPath, statAddr) }) } diff --git a/test/syscalls/linux/stat.cc b/test/syscalls/linux/stat.cc index 553fb7e56..48a2059de 100644 --- a/test/syscalls/linux/stat.cc +++ b/test/syscalls/linux/stat.cc @@ -207,6 +207,98 @@ TEST_F(StatTest, TrailingSlashNotCleanedReturnsENOTDIR) { EXPECT_THAT(lstat(bad_path.c_str(), &buf), SyscallFailsWithErrno(ENOTDIR)); } +// Test fstatating a symlink directory. +TEST_F(StatTest, FstatatSymlinkDir) { + // Create a directory and symlink to it. + const auto dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + + const std::string symlink_to_dir = NewTempAbsPath(); + EXPECT_THAT(symlink(dir.path().c_str(), symlink_to_dir.c_str()), + SyscallSucceeds()); + auto cleanup = Cleanup([&symlink_to_dir]() { + EXPECT_THAT(unlink(symlink_to_dir.c_str()), SyscallSucceeds()); + }); + + // Fstatat the link with AT_SYMLINK_NOFOLLOW should return symlink data. + struct stat st = {}; + EXPECT_THAT( + fstatat(AT_FDCWD, symlink_to_dir.c_str(), &st, AT_SYMLINK_NOFOLLOW), + SyscallSucceeds()); + EXPECT_FALSE(S_ISDIR(st.st_mode)); + EXPECT_TRUE(S_ISLNK(st.st_mode)); + + // Fstatat the link should return dir data. + EXPECT_THAT(fstatat(AT_FDCWD, symlink_to_dir.c_str(), &st, 0), + SyscallSucceeds()); + EXPECT_TRUE(S_ISDIR(st.st_mode)); + EXPECT_FALSE(S_ISLNK(st.st_mode)); +} + +// Test fstatating a symlink directory with trailing slash. +TEST_F(StatTest, FstatatSymlinkDirWithTrailingSlash) { + // Create a directory and symlink to it. + const auto dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + const std::string symlink_to_dir = NewTempAbsPath(); + EXPECT_THAT(symlink(dir.path().c_str(), symlink_to_dir.c_str()), + SyscallSucceeds()); + auto cleanup = Cleanup([&symlink_to_dir]() { + EXPECT_THAT(unlink(symlink_to_dir.c_str()), SyscallSucceeds()); + }); + + // Fstatat on the symlink with a trailing slash should return the directory + // data. + struct stat st = {}; + EXPECT_THAT( + fstatat(AT_FDCWD, absl::StrCat(symlink_to_dir, "/").c_str(), &st, 0), + SyscallSucceeds()); + EXPECT_TRUE(S_ISDIR(st.st_mode)); + EXPECT_FALSE(S_ISLNK(st.st_mode)); + + // Fstatat on the symlink with a trailing slash with AT_SYMLINK_NOFOLLOW + // should return the directory data. + // Symlink to directory with trailing slash will ignore AT_SYMLINK_NOFOLLOW. + EXPECT_THAT(fstatat(AT_FDCWD, absl::StrCat(symlink_to_dir, "/").c_str(), &st, + AT_SYMLINK_NOFOLLOW), + SyscallSucceeds()); + EXPECT_TRUE(S_ISDIR(st.st_mode)); + EXPECT_FALSE(S_ISLNK(st.st_mode)); +} + +// Test fstatating a symlink directory with a trailing slash +// should return same stat data with fstatating directory. +TEST_F(StatTest, FstatatSymlinkDirWithTrailingSlashSameInode) { + // Create a directory and symlink to it. + const auto dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + + // We are going to assert that the symlink inode id is the same as the linked + // dir's inode id. In order for the inode id to be stable across + // save/restore, it must be kept open. The FileDescriptor type will do that + // for us automatically. + auto fd = ASSERT_NO_ERRNO_AND_VALUE(Open(dir.path(), O_RDONLY | O_DIRECTORY)); + + const std::string symlink_to_dir = NewTempAbsPath(); + EXPECT_THAT(symlink(dir.path().c_str(), symlink_to_dir.c_str()), + SyscallSucceeds()); + auto cleanup = Cleanup([&symlink_to_dir]() { + EXPECT_THAT(unlink(symlink_to_dir.c_str()), SyscallSucceeds()); + }); + + // Fstatat on the symlink with a trailing slash should return the directory + // data. + struct stat st = {}; + EXPECT_THAT(fstatat(AT_FDCWD, absl::StrCat(symlink_to_dir, "/").c_str(), &st, + AT_SYMLINK_NOFOLLOW), + SyscallSucceeds()); + EXPECT_TRUE(S_ISDIR(st.st_mode)); + + // Dir and symlink should point to same inode. + struct stat st_dir = {}; + EXPECT_THAT( + fstatat(AT_FDCWD, dir.path().c_str(), &st_dir, AT_SYMLINK_NOFOLLOW), + SyscallSucceeds()); + EXPECT_EQ(st.st_ino, st_dir.st_ino); +} + TEST_F(StatTest, LeadingDoubleSlash) { // Create a file, and make sure we can stat it. TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());