upstream/opnsense-src
1
0
Fork 0
mirror of https://github.com/opnsense/src.git synced 2026-06-14 19:20:18 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

fusefs: add more checks for buggy FUSE servers

Browse source 
* 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
This commit is contained in:
Alan Somers 2025-01-15 12:25:34 -07:00
parent 16587f60a6
commit f1ec3bc06e
6 changed files with 149 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
sys/fs/fuse/fuse_ipc.h
View file

@ -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)

19
sys/fs/fuse/fuse_node.c
View file

@ -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) {

13
sys/fs/fuse/fuse_vfsops.c
View file

@ -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)

5
tests/sys/fs/fusefs/last_local_modify.cc
View file

@ -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;

7
tests/sys/fs/fusefs/lookup.cc
View file

@ -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;
})));

106
tests/sys/fs/fusefs/nfs.cc
View file

@ -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;