The branch main has been updated by asomers:
URL:
https://cgit.FreeBSD.org/src/commit/?id=f1ec3bc06ed276e6e24996515bdce729d51e11d8
commit f1ec3bc06ed276e6e24996515bdce729d51e11d8
Author: Alan Somers <asom...@freebsd.org>
AuthorDate: 2025-01-15 19:25:34 +0000
Commit: Alan Somers <asom...@freebsd.org>
CommitDate: 2025-06-13 21:00:25 +0000
fusefs: add more checks for buggy FUSE servers
* If a FUSE file system is NFS-exported (either by a kernel or userspace
NFS server), then it must support FUSE_LOOKUP operations for ".". But
if the response reports a different nodeid than the request, that's
very bad. Fail the operation and warn the operator.
* In general, a FUSE file may have a distinct "nodeid" and "inode
number". But it the file system is NFS-exported (either by a kernel
or userspace NFS server), then those two must match, because the NFS
server will do VFS_VGET operations using the inode number. If they
don't match, warn the operator.
MFC after: 2 weeks
Sponsored by: ConnectWise
Differential Revision: https://reviews.freebsd.org/D48471
---
sys/fs/fuse/fuse_ipc.h | 2 +
sys/fs/fuse/fuse_node.c | 19 ++++++
sys/fs/fuse/fuse_vfsops.c | 13 ++++
tests/sys/fs/fusefs/last_local_modify.cc | 5 +-
tests/sys/fs/fusefs/lookup.cc | 7 ++
tests/sys/fs/fusefs/nfs.cc | 106 +++++++++++++++++++++++++++++++
6 files changed, 149 insertions(+), 3 deletions(-)
diff --git a/sys/fs/fuse/fuse_ipc.h b/sys/fs/fuse/fuse_ipc.h
index 5648624f4c63..3bfc859dbac9 100644
--- a/sys/fs/fuse/fuse_ipc.h
+++ b/sys/fs/fuse/fuse_ipc.h
@@ -238,6 +238,8 @@ struct fuse_data {
#define FSESS_WARN_WB_CACHE_INCOHERENT 0x400000 /* WB cache incoherent
*/
#define FSESS_WARN_ILLEGAL_INODE 0x800000 /* Illegal inode for new
file */
#define FSESS_WARN_READLINK_EMBEDDED_NUL 0x1000000 /* corrupt READLINK output
*/
+#define FSESS_WARN_DOT_LOOKUP 0x2000000 /* Inconsistent . LOOKUP response */
+#define FSESS_WARN_INODE_MISMATCH 0x4000000 /* ino != nodeid */
#define FSESS_MNTOPTS_MASK ( \
FSESS_DAEMON_CAN_SPY | FSESS_PUSH_SYMLINKS_IN | \
FSESS_DEFAULT_PERMISSIONS | FSESS_INTR)
diff --git a/sys/fs/fuse/fuse_node.c b/sys/fs/fuse/fuse_node.c
index 0a24d0da4fac..742dc66bcafc 100644
--- a/sys/fs/fuse/fuse_node.c
+++ b/sys/fs/fuse/fuse_node.c
@@ -297,6 +297,8 @@ fuse_vnode_get(struct mount *mp,
__enum_uint8(vtype) vtyp)
{
struct thread *td = curthread;
+ bool exportable = fuse_get_mpdata(mp)->dataflags & FSESS_EXPORT_SUPPORT;
+
/*
* feo should only be NULL for the root directory, which (when libfuse
* is used) always has generation 0
@@ -309,6 +311,23 @@ fuse_vnode_get(struct mount *mp,
"Assigned same inode to both parent and child.");
return EIO;
}
+ if (feo && feo->nodeid != feo->attr.ino && exportable) {
+ /*
+ * NFS servers (both kernelspace and userspace) rely on
+ * VFS_VGET to lookup inodes. But that's only possible if the
+ * file's inode number matches its nodeid, which isn't
+ * necessarily the case for FUSE. If they don't match, then we
+ * can complete the current operation, but future VFS_VGET
+ * operations will almost certainly return spurious results.
+ * Warn the operator.
+ *
+ * But only warn the operator if the file system reports
+ * NFS-compatibility, because that's the only time that this
+ * matters, and dumb fuse servers abound.
+ */
+ fuse_warn(fuse_get_mpdata(mp), FSESS_WARN_INODE_MISMATCH,
+ "file has different inode number and nodeid.");
+ }
err = fuse_vnode_alloc(mp, td, nodeid, vtyp, vpp);
if (err) {
diff --git a/sys/fs/fuse/fuse_vfsops.c b/sys/fs/fuse/fuse_vfsops.c
index 0da51b865873..48b84d3c75af 100644
--- a/sys/fs/fuse/fuse_vfsops.c
+++ b/sys/fs/fuse/fuse_vfsops.c
@@ -568,12 +568,25 @@ fuse_vfsop_vget(struct mount *mp, ino_t ino, int flags,
struct vnode **vpp)
goto out;
feo = (struct fuse_entry_out *)fdi.answ;
+
if (feo->nodeid == 0) {
/* zero nodeid means ENOENT and cache it */
error = ENOENT;
goto out;
}
+ if (feo->nodeid != nodeid) {
+ /*
+ * Something is very wrong with the server if "foo/." has a
+ * different inode number than "foo".
+ */
+ fuse_warn(data, FSESS_WARN_DOT_LOOKUP,
+ "Inconsistent LOOKUP response: \"FILE/.\" has a different "
+ "inode number than \"FILE\".");
+ error = EIO;
+ goto out;
+ }
+
vtyp = IFTOVT(feo->attr.mode);
error = fuse_vnode_get(mp, feo, nodeid, NULL, vpp, NULL, vtyp);
if (error)
diff --git a/tests/sys/fs/fusefs/last_local_modify.cc
b/tests/sys/fs/fusefs/last_local_modify.cc
index 495bfd8aa959..5fcd3c36c892 100644
--- a/tests/sys/fs/fusefs/last_local_modify.cc
+++ b/tests/sys/fs/fusefs/last_local_modify.cc
@@ -233,7 +233,6 @@ TEST_P(LastLocalModify, lookup)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.nodeid = ino;
out.body.entry.attr.size = oldsize;
- out.body.entry.nodeid = ino;
out.body.entry.attr_valid_nsec = NAP_NS / 2;
out.body.entry.attr.ino = ino;
out.body.entry.attr.mode = S_IFREG | 0644;
@@ -277,6 +276,7 @@ TEST_P(LastLocalModify, lookup)
SET_OUT_HEADER_LEN(*out0, entry);
out0->body.entry.attr.mode = S_IFREG | 0644;
out0->body.entry.nodeid = ino;
+ out0->body.entry.attr.ino = ino;
out0->body.entry.entry_valid = UINT64_MAX;
out0->body.entry.attr_valid = UINT64_MAX;
out0->body.entry.attr.size = oldsize;
@@ -392,7 +392,6 @@ TEST_P(LastLocalModify, vfs_vget)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.nodeid = ino;
out.body.entry.attr.size = oldsize;
- out.body.entry.nodeid = ino;
out.body.entry.attr_valid_nsec = NAP_NS / 2;
out.body.entry.attr.ino = ino;
out.body.entry.attr.mode = S_IFREG | 0644;
@@ -414,7 +413,6 @@ TEST_P(LastLocalModify, vfs_vget)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.nodeid = ino;
out.body.entry.attr.size = oldsize;
- out.body.entry.nodeid = ino;
out.body.entry.attr_valid_nsec = NAP_NS / 2;
out.body.entry.attr.ino = ino;
out.body.entry.attr.mode = S_IFREG | 0644;
@@ -439,6 +437,7 @@ TEST_P(LastLocalModify, vfs_vget)
SET_OUT_HEADER_LEN(*out0, entry);
out0->body.entry.attr.mode = S_IFREG | 0644;
out0->body.entry.nodeid = ino;
+ out0->body.entry.attr.ino = ino;
out0->body.entry.entry_valid = UINT64_MAX;
out0->body.entry.attr_valid = UINT64_MAX;
out0->body.entry.attr.size = oldsize;
diff --git a/tests/sys/fs/fusefs/lookup.cc b/tests/sys/fs/fusefs/lookup.cc
index 6d506c1ab700..2cfe888b6b08 100644
--- a/tests/sys/fs/fusefs/lookup.cc
+++ b/tests/sys/fs/fusefs/lookup.cc
@@ -560,6 +560,7 @@ TEST_F(LookupExportable, dotdot_entry_cache_timeout)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = S_IFDIR | 0755;
out.body.entry.nodeid = foo_ino;
+ out.body.entry.attr.ino = foo_ino;
out.body.entry.attr_valid = UINT64_MAX;
out.body.entry.entry_valid = 0; // immediate timeout
})));
@@ -568,6 +569,7 @@ TEST_F(LookupExportable, dotdot_entry_cache_timeout)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = S_IFDIR | 0755;
out.body.entry.nodeid = bar_ino;
+ out.body.entry.attr.ino = bar_ino;
out.body.entry.attr_valid = UINT64_MAX;
out.body.entry.entry_valid = UINT64_MAX;
})));
@@ -577,6 +579,7 @@ TEST_F(LookupExportable, dotdot_entry_cache_timeout)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = S_IFDIR | 0755;
out.body.entry.nodeid = FUSE_ROOT_ID;
+ out.body.entry.attr.ino = FUSE_ROOT_ID;
out.body.entry.attr_valid = UINT64_MAX;
out.body.entry.entry_valid = UINT64_MAX;
})));
@@ -607,6 +610,7 @@ TEST_F(LookupExportable, dotdot_no_parent_nid)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = S_IFDIR | 0755;
out.body.entry.nodeid = foo_ino;
+ out.body.entry.attr.ino = foo_ino;
out.body.entry.attr_valid = UINT64_MAX;
out.body.entry.entry_valid = UINT64_MAX;
})));
@@ -615,6 +619,7 @@ TEST_F(LookupExportable, dotdot_no_parent_nid)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = S_IFDIR | 0755;
out.body.entry.nodeid = bar_ino;
+ out.body.entry.attr.ino = bar_ino;
out.body.entry.attr_valid = UINT64_MAX;
out.body.entry.entry_valid = UINT64_MAX;
})));
@@ -632,6 +637,7 @@ TEST_F(LookupExportable, dotdot_no_parent_nid)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = S_IFDIR | 0755;
out.body.entry.nodeid = foo_ino;
+ out.body.entry.attr.ino = foo_ino;
out.body.entry.attr_valid = UINT64_MAX;
out.body.entry.entry_valid = UINT64_MAX;
})));
@@ -640,6 +646,7 @@ TEST_F(LookupExportable, dotdot_no_parent_nid)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = S_IFDIR | 0755;
out.body.entry.nodeid = FUSE_ROOT_ID;
+ out.body.entry.attr.ino = FUSE_ROOT_ID;
out.body.entry.attr_valid = UINT64_MAX;
out.body.entry.entry_valid = UINT64_MAX;
})));
diff --git a/tests/sys/fs/fusefs/nfs.cc b/tests/sys/fs/fusefs/nfs.cc
index 27ffc8f5cbc1..2fa2b290f383 100644
--- a/tests/sys/fs/fusefs/nfs.cc
+++ b/tests/sys/fs/fusefs/nfs.cc
@@ -84,6 +84,7 @@ TEST_F(Fhstat, estale)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = mode;
out.body.entry.nodeid = ino;
+ out.body.entry.attr.ino = ino;
out.body.entry.generation = 1;
out.body.entry.attr_valid = UINT64_MAX;
out.body.entry.entry_valid = 0;
@@ -95,6 +96,7 @@ TEST_F(Fhstat, estale)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = mode;
out.body.entry.nodeid = ino;
+ out.body.entry.attr.ino = ino;
out.body.entry.generation = 2;
out.body.entry.attr_valid = UINT64_MAX;
out.body.entry.entry_valid = 0;
@@ -121,6 +123,7 @@ TEST_F(Fhstat, lookup_dot)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = mode;
out.body.entry.nodeid = ino;
+ out.body.entry.attr.ino = ino;
out.body.entry.generation = 1;
out.body.entry.attr.uid = uid;
out.body.entry.attr_valid = UINT64_MAX;
@@ -132,6 +135,7 @@ TEST_F(Fhstat, lookup_dot)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = mode;
out.body.entry.nodeid = ino;
+ out.body.entry.attr.ino = ino;
out.body.entry.generation = 1;
out.body.entry.attr.uid = uid;
out.body.entry.attr_valid = UINT64_MAX;
@@ -160,6 +164,7 @@ TEST_F(Fhstat, lookup_dot_error)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = mode;
out.body.entry.nodeid = ino;
+ out.body.entry.attr.ino = ino;
out.body.entry.generation = 1;
out.body.entry.attr.uid = uid;
out.body.entry.attr_valid = UINT64_MAX;
@@ -189,6 +194,7 @@ TEST_F(Fhstat, cached)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = mode;
out.body.entry.nodeid = ino;
+ out.body.entry.attr.ino = ino;
out.body.entry.generation = 1;
out.body.entry.attr.ino = ino;
out.body.entry.attr_valid = UINT64_MAX;
@@ -215,6 +221,7 @@ TEST_F(Fhstat, cache_expired)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = mode;
out.body.entry.nodeid = ino;
+ out.body.entry.attr.ino = ino;
out.body.entry.generation = 1;
out.body.entry.attr.ino = ino;
out.body.entry.attr_valid = UINT64_MAX;
@@ -226,6 +233,7 @@ TEST_F(Fhstat, cache_expired)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = mode;
out.body.entry.nodeid = ino;
+ out.body.entry.attr.ino = ino;
out.body.entry.generation = 1;
out.body.entry.attr.ino = ino;
out.body.entry.attr_valid = UINT64_MAX;
@@ -243,6 +251,99 @@ TEST_F(Fhstat, cache_expired)
EXPECT_EQ(ino, sb.st_ino);
}
+/*
+ * If the server returns a FUSE_LOOKUP response for a nodeid that we didn't
+ * lookup, it's a bug. But we should handle it gracefully.
+ */
+TEST_F(Fhstat, inconsistent_nodeid)
+{
+ const char FULLPATH[] = "mountpoint/some_dir/.";
+ const char RELDIRPATH[] = "some_dir";
+ fhandle_t fhp;
+ struct stat sb;
+ const uint64_t ino_in = 42;
+ const uint64_t ino_out = 43;
+ const mode_t mode = S_IFDIR | 0755;
+ const uid_t uid = 12345;
+
+ EXPECT_LOOKUP(FUSE_ROOT_ID, RELDIRPATH)
+ .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
+ SET_OUT_HEADER_LEN(out, entry);
+ out.body.entry.nodeid = ino_in;
+ out.body.entry.attr.ino = ino_in;
+ out.body.entry.attr.mode = mode;
+ out.body.entry.generation = 1;
+ out.body.entry.attr.uid = uid;
+ out.body.entry.attr_valid = UINT64_MAX;
+ out.body.entry.entry_valid = 0;
+ })));
+
+ EXPECT_LOOKUP(ino_in, ".")
+ .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
+ SET_OUT_HEADER_LEN(out, entry);
+ out.body.entry.nodeid = ino_out;
+ out.body.entry.attr.ino = ino_out;
+ out.body.entry.attr.mode = mode;
+ out.body.entry.generation = 1;
+ out.body.entry.attr.uid = uid;
+ out.body.entry.attr_valid = UINT64_MAX;
+ out.body.entry.entry_valid = 0;
+ })));
+
+ ASSERT_EQ(0, getfh(FULLPATH, &fhp)) << strerror(errno);
+ EXPECT_NE(0, fhstat(&fhp, &sb)) << strerror(errno);
+ EXPECT_EQ(EIO, errno);
+}
+
+/*
+ * If the server returns a FUSE_LOOKUP response where the nodeid doesn't match
+ * the inode number, and the file system is exported, it's a bug. But we
+ * should handle it gracefully.
+ */
+TEST_F(Fhstat, inconsistent_ino)
+{
+ const char FULLPATH[] = "mountpoint/some_dir/.";
+ const char RELDIRPATH[] = "some_dir";
+ fhandle_t fhp;
+ struct stat sb;
+ const uint64_t nodeid = 42;
+ const uint64_t ino = 711; // Could be anything that != nodeid
+ const mode_t mode = S_IFDIR | 0755;
+ const uid_t uid = 12345;
+
+ EXPECT_LOOKUP(FUSE_ROOT_ID, RELDIRPATH)
+ .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
+ SET_OUT_HEADER_LEN(out, entry);
+ out.body.entry.nodeid = nodeid;
+ out.body.entry.attr.ino = nodeid;
+ out.body.entry.attr.mode = mode;
+ out.body.entry.generation = 1;
+ out.body.entry.attr.uid = uid;
+ out.body.entry.attr_valid = UINT64_MAX;
+ out.body.entry.entry_valid = 0;
+ })));
+
+ EXPECT_LOOKUP(nodeid, ".")
+ .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
+ SET_OUT_HEADER_LEN(out, entry);
+ out.body.entry.nodeid = nodeid;
+ out.body.entry.attr.ino = ino;
+ out.body.entry.attr.mode = mode;
+ out.body.entry.generation = 1;
+ out.body.entry.attr.uid = uid;
+ out.body.entry.attr_valid = UINT64_MAX;
+ out.body.entry.entry_valid = 0;
+ })));
+
+ ASSERT_EQ(0, getfh(FULLPATH, &fhp)) << strerror(errno);
+ /*
+ * The fhstat operation will actually succeed. But future operations
+ * will likely fail.
+ */
+ ASSERT_EQ(0, fhstat(&fhp, &sb)) << strerror(errno);
+ EXPECT_EQ(ino, sb.st_ino);
+}
+
/*
* If the server doesn't set FUSE_EXPORT_SUPPORT, then we can't do NFS-style
* lookups
@@ -260,6 +361,7 @@ TEST_F(FhstatNotExportable, lookup_dot)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = mode;
out.body.entry.nodeid = ino;
+ out.body.entry.attr.ino = ino;
out.body.entry.generation = 1;
out.body.entry.attr_valid = UINT64_MAX;
out.body.entry.entry_valid = 0;
@@ -282,6 +384,7 @@ TEST_F(Getfh, eoverflow)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = S_IFDIR | 0755;
out.body.entry.nodeid = ino;
+ out.body.entry.attr.ino = ino;
out.body.entry.generation = (uint64_t)UINT32_MAX + 1;
out.body.entry.attr_valid = UINT64_MAX;
out.body.entry.entry_valid = UINT64_MAX;
@@ -304,6 +407,7 @@ TEST_F(Getfh, ok)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = S_IFDIR | 0755;
out.body.entry.nodeid = ino;
+ out.body.entry.attr.ino = ino;
out.body.entry.attr_valid = UINT64_MAX;
out.body.entry.entry_valid = UINT64_MAX;
})));
@@ -335,6 +439,7 @@ TEST_F(Readdir, getdirentries)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = mode;
out.body.entry.nodeid = ino;
+ out.body.entry.attr.ino = ino;
out.body.entry.generation = 1;
out.body.entry.attr_valid = UINT64_MAX;
out.body.entry.entry_valid = 0;
@@ -345,6 +450,7 @@ TEST_F(Readdir, getdirentries)
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = mode;
out.body.entry.nodeid = ino;
+ out.body.entry.attr.ino = ino;
out.body.entry.generation = 1;
out.body.entry.attr_valid = UINT64_MAX;
out.body.entry.entry_valid = 0;
