diff --git a/pkg/sentry/fs/inode_overlay.go b/pkg/sentry/fs/inode_overlay.go index 24b769cfc..e0602da17 100644 --- a/pkg/sentry/fs/inode_overlay.go +++ b/pkg/sentry/fs/inode_overlay.go @@ -339,7 +339,9 @@ func overlayRemove(ctx context.Context, o *overlayEntry, parent *Dirent, child * } } if child.Inode.overlay.lowerExists { - return overlayCreateWhiteout(o.upper, child.name) + if err := overlayCreateWhiteout(o.upper, child.name); err != nil { + return err + } } // We've removed from the directory so we must drop the cache. o.markDirectoryDirty() @@ -418,10 +420,12 @@ func overlayRename(ctx context.Context, o *overlayEntry, oldParent *Dirent, rena return err } if renamed.Inode.overlay.lowerExists { - return overlayCreateWhiteout(oldParent.Inode.overlay.upper, oldName) + if err := overlayCreateWhiteout(oldParent.Inode.overlay.upper, oldName); err != nil { + return err + } } // We've changed the directory so we must drop the cache. - o.markDirectoryDirty() + oldParent.Inode.overlay.markDirectoryDirty() return nil } diff --git a/test/syscalls/linux/getdents.cc b/test/syscalls/linux/getdents.cc index 8e4efa8d6..fe9cfafe8 100644 --- a/test/syscalls/linux/getdents.cc +++ b/test/syscalls/linux/getdents.cc @@ -40,6 +40,7 @@ #include "test/util/temp_path.h" #include "test/util/test_util.h" +using ::testing::Contains; using ::testing::IsEmpty; using ::testing::IsSupersetOf; using ::testing::Not; @@ -484,6 +485,44 @@ TEST(ReaddirTest, Bug35110122Root) { EXPECT_THAT(contents, Not(IsEmpty())); } +// Unlink should invalidate getdents cache. +TEST(ReaddirTest, GoneAfterRemoveCache) { + TempPath dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileIn(dir.path())); + std::string name = std::string(Basename(file.path())); + + auto contents = ASSERT_NO_ERRNO_AND_VALUE(ListDir(dir.path(), true)); + EXPECT_THAT(contents, Contains(name)); + + file.reset(); + + contents = ASSERT_NO_ERRNO_AND_VALUE(ListDir(dir.path(), true)); + EXPECT_THAT(contents, Not(Contains(name))); +} + +// Regression test for b/137398511. Rename should invalidate getdents cache. +TEST(ReaddirTest, GoneAfterRenameCache) { + TempPath src = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + TempPath dst = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + + TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileIn(src.path())); + std::string name = std::string(Basename(file.path())); + + auto contents = ASSERT_NO_ERRNO_AND_VALUE(ListDir(src.path(), true)); + EXPECT_THAT(contents, Contains(name)); + + ASSERT_THAT(rename(file.path().c_str(), JoinPath(dst.path(), name).c_str()), + SyscallSucceeds()); + // Release file since it was renamed. dst cleanup will ultimately delete it. + file.release(); + + contents = ASSERT_NO_ERRNO_AND_VALUE(ListDir(src.path(), true)); + EXPECT_THAT(contents, Not(Contains(name))); + + contents = ASSERT_NO_ERRNO_AND_VALUE(ListDir(dst.path(), true)); + EXPECT_THAT(contents, Contains(name)); +} + } // namespace } // namespace testing