From 8a499ae65f361fb01c2e4be03122f69910a8ba4a Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Mon, 18 Mar 2019 18:39:08 -0700 Subject: 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 --- test/syscalls/linux/rename.cc | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) (limited to 'test/syscalls') 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"); -- cgit v1.2.3