go
/
gvisor
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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
This commit is contained in:
Michael Pratt 2019-07-17 11:13:37 -07:00 committed by gVisor bot
parent 78a2704bde
commit ca829158e3
2 changed files with 46 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
pkg/sentry/fs/inode_overlay.go
View File

@ -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
}

39
test/syscalls/linux/getdents.cc
View File

@ -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