Page MenuHomeFreeBSD
Files F107682943

D33283.diff
No OneTemporary
Actions

D33283.diff
View Options

diff --git a/sys/fs/fuse/fuse_internal.c b/sys/fs/fuse/fuse_internal.c
--- a/sys/fs/fuse/fuse_internal.c
+++ b/sys/fs/fuse/fuse_internal.c
@@ -1255,12 +1255,15 @@
* STALE vnode, ditch
*
* The vnode has changed its type "behind our back".
+ * This probably means that the file got deleted and
+ * recreated on the server, with the same inode.
* There's nothing really we can do, so let us just
- * force an internal revocation and tell the caller to
- * try again, if interested.
+ * return ENOENT. After all, the entry must not have
+ * existed in the recent past. If the user tries
+ * again, it will work.
*/
fuse_internal_vnode_disappear(vp);
- err = EAGAIN;
+ err = ENOENT;
}
}
if (err == 0) {
diff --git a/sys/fs/fuse/fuse_node.c b/sys/fs/fuse/fuse_node.c
--- a/sys/fs/fuse/fuse_node.c
+++ b/sys/fs/fuse/fuse_node.c
@@ -213,24 +213,27 @@
return (err);
if (*vpp) {
- if ((*vpp)->v_type != vtyp) {
+ if ((*vpp)->v_type == vtyp) {
+ /* Reuse a vnode that hasn't yet been reclaimed */
+ MPASS((*vpp)->v_data != NULL);
+ MPASS(VTOFUD(*vpp)->nid == nodeid);
+ SDT_PROBE2(fusefs, , node, trace, 1,
+ "vnode taken from hash");
+ return (0);
+ } else {
/*
- * STALE vnode! This probably indicates a buggy
- * server, but it could also be the result of a race
- * between FUSE_LOOKUP and another client's
- * FUSE_UNLINK/FUSE_CREATE
+ * The inode changed types! If we get here, we can't
+ * tell whether the inode's entry cache had expired
+ * yet. So this could be the result of a buggy server,
+ * but more likely the server just reused an inode
+ * number following an entry cache expiration.
*/
SDT_PROBE3(fusefs, , node, stale_vnode, *vpp, vtyp,
nodeid);
fuse_internal_vnode_disappear(*vpp);
+ vgone(*vpp);
lockmgr((*vpp)->v_vnlock, LK_RELEASE, NULL);
- *vpp = NULL;
- return (EAGAIN);
}
- MPASS((*vpp)->v_data != NULL);
- MPASS(VTOFUD(*vpp)->nid == nodeid);
- SDT_PROBE2(fusefs, , node, trace, 1, "vnode taken from hash");
- return (0);
}
fvdat = malloc(sizeof(*fvdat), M_FUSEVN, M_WAITOK | M_ZERO);
switch (vtyp) {
diff --git a/tests/sys/fs/fusefs/getattr.cc b/tests/sys/fs/fusefs/getattr.cc
--- a/tests/sys/fs/fusefs/getattr.cc
+++ b/tests/sys/fs/fusefs/getattr.cc
@@ -256,6 +256,56 @@
//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
--- a/tests/sys/fs/fusefs/lookup.cc
+++ b/tests/sys/fs/fusefs/lookup.cc
@@ -342,9 +342,10 @@
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 @@
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
--- a/tests/sys/fs/fusefs/setattr.cc
+++ b/tests/sys/fs/fusefs/setattr.cc
@@ -724,6 +724,53 @@
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)
{

File Metadata

Mime Type
text/plain
Expires
Sat, Jan 18, 1:33 PM (16 h, 46 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
15862000
Default Alt Text
D33283.diff (7 KB)

Event Timeline