Properly invalidate cache in rename and remove

We were invalidating the wrong overlayEntry in rename and missing invalidation
in rename and remove if lower exists.

PiperOrigin-RevId: 258604685

2 files changed, 46 insertions, 3 deletions

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