diff options
author | Michael Pratt <mpratt@google.com> | 2019-03-18 18:39:08 -0700 |
---|---|---|
committer | Shentubot <shentubot@google.com> | 2019-03-18 18:40:06 -0700 |
commit | 8a499ae65f361fb01c2e4be03122f69910a8ba4a (patch) | |
tree | 6b217045a189f94b9bd62756fe61bf40f34d622f /test | |
parent | e420cc3e5d2066674d32d16ad885bee6b30da210 (diff) |
Remove references to replaced child in Rename in ramfs/agentfs
In the case of a rename replacing an existing destination inode, ramfs
Rename failed to first remove the replaced inode. This caused:
1. A leak of a reference to the inode (making it live indefinitely).
2. For directories, a leak of the replaced directory's .. link to the
parent. This would cause the parent's link count to incorrectly
increase.
(2) is much simpler to test than (1), so that's what I've done.
agentfs has a similar bug with link count only, so the Dirent layer
informs the Inode if this is a replacing rename.
Fixes #133
PiperOrigin-RevId: 239105698
Change-Id: I4450af2462d8ae3339def812287213d2cbeebde0
Diffstat (limited to 'test')
-rw-r--r-- | test/BUILD (renamed from test/BUILD.opensource) | 2 | ||||
-rw-r--r-- | test/syscalls/linux/rename.cc | 25 |
2 files changed, 24 insertions, 3 deletions
diff --git a/test/BUILD.opensource b/test/BUILD index 8d2969204..6b83757f6 100644 --- a/test/BUILD.opensource +++ b/test/BUILD @@ -1,6 +1,6 @@ # gVisor is a general-purpose sandbox. -licenses(["notice"]) +package(licenses = ["notice"]) exports_files(["LICENSE"]) diff --git a/test/syscalls/linux/rename.cc b/test/syscalls/linux/rename.cc index f4c877a00..c0cbc7cd9 100644 --- a/test/syscalls/linux/rename.cc +++ b/test/syscalls/linux/rename.cc @@ -155,10 +155,11 @@ TEST(RenameTest, DirectoryToOwnChildDirectory) { } TEST(RenameTest, FileOverwritesFile) { + auto dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); auto f1 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith( - GetAbsoluteTestTmpdir(), "first", TempPath::kDefaultFileMode)); + dir.path(), "first", TempPath::kDefaultFileMode)); auto f2 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith( - GetAbsoluteTestTmpdir(), "second", TempPath::kDefaultFileMode)); + dir.path(), "second", TempPath::kDefaultFileMode)); ASSERT_THAT(rename(f1.path().c_str(), f2.path().c_str()), SyscallSucceeds()); EXPECT_THAT(Exists(f1.path()), IsPosixErrorOkAndHolds(false)); @@ -168,6 +169,26 @@ TEST(RenameTest, FileOverwritesFile) { EXPECT_EQ("first", f2_contents); } +TEST(RenameTest, DirectoryOverwritesDirectoryLinkCount) { + auto parent1 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + EXPECT_THAT(Links(parent1.path()), IsPosixErrorOkAndHolds(2)); + + auto parent2 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + EXPECT_THAT(Links(parent2.path()), IsPosixErrorOkAndHolds(2)); + + auto dir1 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(parent1.path())); + auto dir2 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(parent2.path())); + + EXPECT_THAT(Links(parent1.path()), IsPosixErrorOkAndHolds(3)); + EXPECT_THAT(Links(parent2.path()), IsPosixErrorOkAndHolds(3)); + + ASSERT_THAT(rename(dir1.path().c_str(), dir2.path().c_str()), + SyscallSucceeds()); + + EXPECT_THAT(Links(parent1.path()), IsPosixErrorOkAndHolds(2)); + EXPECT_THAT(Links(parent2.path()), IsPosixErrorOkAndHolds(3)); +} + TEST(RenameTest, FileDoesNotExist) { auto dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); const std::string source = JoinPath(dir.path(), "source"); |