diff options
author | Alan Somers <asomers@FreeBSD.org> | 2021-12-06 05:43:17 +0000 |
---|---|---|
committer | Alan Somers <asomers@FreeBSD.org> | 2021-12-07 04:36:46 +0000 |
commit | 25927e068fcbcac0a5111a881de723bd984b04b3 (patch) | |
tree | 649e9e14365899a71564eabe9803aa1de1a5473d /tests | |
parent | 169b368a62aac38091a302b2970df81e0281e98f (diff) | |
download | src-25927e068fcbcac0a5111a881de723bd984b04b3.tar.gz src-25927e068fcbcac0a5111a881de723bd984b04b3.zip |
fusefs: correctly handle an inode that changes file types
Correctly handle the situation where a FUSE server unlinks a file, then
creates a new file of a different type but with the same inode number.
Previously fuse_vnop_lookup in this situation would return EAGAIN. But
since it didn't call vgone(), the vnode couldn't be reused right away.
Fix this by immediately calling vgone() and reallocating a new vnode.
This problem can occur in three code paths, during VOP_LOOKUP,
VOP_SETATTR, or following FUSE_GETATTR, which usually happens during
VOP_GETATTR but can occur during other vops, too. Note that the correct
response actually doesn't depend on whether the entry cache has expired.
In fact, during VOP_LOOKUP, we can't even tell. Either it has expired
already, or else the vnode got reclaimed by vnlru.
Also, correct the error code during the VOP_SETATTR path.
PR: 258022
Reported by: chogata@moosefs.pro
MFC after: 2 weeks
Reviewed by: pfg
Differential Revision: https://reviews.freebsd.org/D33283
Diffstat (limited to 'tests')
-rw-r--r-- | tests/sys/fs/fusefs/getattr.cc | 50 | ||||
-rw-r--r-- | tests/sys/fs/fusefs/lookup.cc | 32 | ||||
-rw-r--r-- | tests/sys/fs/fusefs/setattr.cc | 47 |
3 files changed, 122 insertions, 7 deletions
diff --git a/tests/sys/fs/fusefs/getattr.cc b/tests/sys/fs/fusefs/getattr.cc index fb91f8c049d0..6bca7e0af7c7 100644 --- a/tests/sys/fs/fusefs/getattr.cc +++ b/tests/sys/fs/fusefs/getattr.cc @@ -256,6 +256,56 @@ TEST_F(Getattr, ok) //FUSE can't set st_blksize until protocol 7.9 } +/* + * FUSE_GETATTR returns a different file type, even though the entry cache + * hasn't expired. This is a server bug! It probably means that the server + * removed the file and recreated it with the same inode but a different vtyp. + * The best thing fusefs can do is return ENOENT to the caller. After all, the + * entry must not have existed recently. + */ +TEST_F(Getattr, vtyp_conflict) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + const uint64_t ino = 42; + struct stat sb; + sem_t sem; + + ASSERT_EQ(0, sem_init(&sem, 0, 0)) << strerror(errno); + + EXPECT_LOOKUP(FUSE_ROOT_ID, RELPATH) + .WillOnce(Invoke( + ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, entry); + out.body.entry.attr.mode = S_IFREG | 0644; + out.body.entry.nodeid = ino; + out.body.entry.attr.nlink = 1; + out.body.entry.attr_valid = 0; + out.body.entry.entry_valid = UINT64_MAX; + }))); + EXPECT_CALL(*m_mock, process( + ResultOf([](auto in) { + return (in.header.opcode == FUSE_GETATTR && + in.body.getattr.getattr_flags == 0 && + in.header.nodeid == ino); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnImmediate([](auto i __unused, auto& out) { + SET_OUT_HEADER_LEN(out, attr); + out.body.attr.attr.ino = ino; // Must match nodeid + out.body.attr.attr.mode = S_IFDIR | 0755; // Changed! + out.body.attr.attr.nlink = 2; + }))); + // We should reclaim stale vnodes + expect_forget(ino, 1, &sem); + + ASSERT_NE(0, stat(FULLPATH, &sb)); + EXPECT_EQ(errno, ENOENT); + + sem_wait(&sem); + sem_destroy(&sem); +} + TEST_F(Getattr_7_8, ok) { const char FULLPATH[] = "mountpoint/some_file.txt"; diff --git a/tests/sys/fs/fusefs/lookup.cc b/tests/sys/fs/fusefs/lookup.cc index cb9d0bb27527..d301990c2048 100644 --- a/tests/sys/fs/fusefs/lookup.cc +++ b/tests/sys/fs/fusefs/lookup.cc @@ -342,9 +342,10 @@ TEST_F(Lookup, subdir) ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno); } -/* - * The server returns two different vtypes for the same nodeid. This is a bad - * server! But we shouldn't crash. +/* + * The server returns two different vtypes for the same nodeid. This is + * technically allowed if the entry's cache has already expired. + * https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=258022 */ TEST_F(Lookup, vtype_conflict) { @@ -354,12 +355,29 @@ TEST_F(Lookup, vtype_conflict) const char SECONDRELPATH[] = "bar"; uint64_t ino = 42; - expect_lookup(FIRSTRELPATH, ino, S_IFREG | 0644, 0, 1, UINT64_MAX); - expect_lookup(SECONDRELPATH, ino, S_IFDIR | 0755, 0, 1, UINT64_MAX); + EXPECT_LOOKUP(FUSE_ROOT_ID, FIRSTRELPATH) + .WillOnce(Invoke( + ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, entry); + out.body.entry.attr.mode = S_IFDIR | 0644; + out.body.entry.nodeid = ino; + out.body.entry.attr.nlink = 1; + }))); + expect_lookup(SECONDRELPATH, ino, S_IFREG | 0755, 0, 1, UINT64_MAX); + // VOP_FORGET happens asynchronously, so it may or may not arrive + // before the test completes. + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in.header.opcode == FUSE_FORGET && + in.header.nodeid == ino && + in.body.forget.nlookup == 1); + }, Eq(true)), + _) + ).Times(AtMost(1)) + .WillOnce(Invoke([=](auto in __unused, auto &out __unused) { })); ASSERT_EQ(0, access(FIRSTFULLPATH, F_OK)) << strerror(errno); - ASSERT_EQ(-1, access(SECONDFULLPATH, F_OK)); - ASSERT_EQ(EAGAIN, errno); + EXPECT_EQ(0, access(SECONDFULLPATH, F_OK)) << strerror(errno); } TEST_F(Lookup_7_8, ok) diff --git a/tests/sys/fs/fusefs/setattr.cc b/tests/sys/fs/fusefs/setattr.cc index 48aa8385517f..e4458db9f8ee 100644 --- a/tests/sys/fs/fusefs/setattr.cc +++ b/tests/sys/fs/fusefs/setattr.cc @@ -724,6 +724,53 @@ TEST_F(Setattr, utimensat_utime_now) { EXPECT_EQ(now[1].tv_nsec, sb.st_mtim.tv_nsec); } +/* + * FUSE_SETATTR returns a different file type, even though the entry cache + * hasn't expired. This is a server bug! It probably means that the server + * removed the file and recreated it with the same inode but a different vtyp. + * The best thing fusefs can do is return ENOENT to the caller. After all, the + * entry must not have existed recently. + */ +TEST_F(Setattr, vtyp_conflict) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + const uint64_t ino = 42; + uid_t newuser = 12345; + sem_t sem; + + ASSERT_EQ(0, sem_init(&sem, 0, 0)) << strerror(errno); + + EXPECT_LOOKUP(FUSE_ROOT_ID, RELPATH) + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, entry); + out.body.entry.attr.mode = S_IFREG | 0777; + out.body.entry.nodeid = ino; + out.body.entry.entry_valid = UINT64_MAX; + }))); + + EXPECT_CALL(*m_mock, process( + ResultOf([](auto in) { + return (in.header.opcode == FUSE_SETATTR && + in.header.nodeid == ino); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, attr); + out.body.attr.attr.ino = ino; + out.body.attr.attr.mode = S_IFDIR | 0777; // Changed! + out.body.attr.attr.uid = newuser; + }))); + // We should reclaim stale vnodes + expect_forget(ino, 1, &sem); + + EXPECT_NE(0, chown(FULLPATH, newuser, -1)); + EXPECT_EQ(ENOENT, errno); + + sem_wait(&sem); + sem_destroy(&sem); +} + /* On a read-only mount, no attributes may be changed */ TEST_F(RofsSetattr, erofs) { |