d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fusefs: handle evil servers that return illegal inode numbers

Browse Source 
* If during FUSE_CREATE, FUSE_MKDIR, etc the server returns the same
  inode number for the new file as for its parent directory, reject it.
  Previously this would triggers a recurse-on-non-recursive lock panic.

* If during FUSE_LINK the server returns a different inode number for
  the new name as for the old one, reject it.  Obviously, that can't be
  a hard link.

* If during FUSE_LOOKUP the server returns the same inode number for the
  new file as for its parent directory, reject it.  Nothing good can
  come of this.

PR:		263662
Reported by:	Robert Morris <rtm@lcs.mit.edu>
MFC after:	2 weeks
Reviewed by:	pfg
Differential Revision: https://reviews.freebsd.org/D35128
This commit is contained in:
Alan Somers 2022-05-04 17:36:17 -06:00
parent 8b582b1640
commit 0bef4927ea
9 changed files with 257 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
sys/fs/fuse/fuse_ipc.h
View File

@ -240,6 +240,7 @@ struct fuse_data {
#define FSESS_WARN_LSEXTATTR_LONG 0x100000 /* Returned too many extattrs */
#define FSESS_WARN_CACHE_INCOHERENT 0x200000 /* Read cache incoherent */
#define FSESS_WARN_WB_CACHE_INCOHERENT 0x400000 /* WB cache incoherent */
#define FSESS_WARN_ILLEGAL_INODE 0x800000 /* Illegal inode for new file */
#define FSESS_MNTOPTS_MASK ( \
FSESS_DAEMON_CAN_SPY | FSESS_PUSH_SYMLINKS_IN | \
FSESS_DEFAULT_PERMISSIONS | FSESS_INTR)

6
sys/fs/fuse/fuse_node.c
View File

@ -298,6 +298,12 @@ fuse_vnode_get(struct mount *mp,
uint64_t generation = feo ? feo->generation : 0;
int err = 0;
if (dvp != NULL && VTOFUD(dvp)->nid == nodeid) {
fuse_warn(fuse_get_mpdata(mp), FSESS_WARN_ILLEGAL_INODE,
"Assigned same inode to both parent and child.");
return EIO;
}
err = fuse_vnode_alloc(mp, td, nodeid, vtyp, vpp);
if (err) {
return err;

27
sys/fs/fuse/fuse_vnops.c
View File

@ -1327,6 +1327,16 @@ fuse_vnop_link(struct vop_link_args *ap)
}
feo = fdi.answ;
if (fli.oldnodeid != feo->nodeid) {
struct fuse_data *data = fuse_get_mpdata(vnode_mount(vp));
fuse_warn(data, FSESS_WARN_ILLEGAL_INODE,
"Assigned wrong inode for a hard link.");
fuse_vnode_clear_attr_cache(vp);
fuse_vnode_clear_attr_cache(tdvp);
err = EIO;
goto out;
}
err = fuse_internal_checkentry(feo, vnode_vtype(vp));
if (!err) {
/*
@ -1386,6 +1396,7 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
struct mount *mp = vnode_mount(dvp);
struct fuse_data *data = fuse_get_mpdata(mp);
int default_permissions = data->dataflags & FSESS_DEFAULT_PERMISSIONS;
bool is_dot;
int err = 0;
int lookup_err = 0;
@ -1413,6 +1424,7 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
else if ((err = fuse_internal_access(dvp, VEXEC, td, cred)))
return err;
is_dot = cnp->cn_namelen == 1 && *(cnp->cn_nameptr) == '.';
if ((flags & ISDOTDOT) && !(data->dataflags & FSESS_EXPORT_SUPPORT))
{
if (!(VTOFUD(dvp)->flag & FN_PARENT_NID)) {
@ -1427,7 +1439,7 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
return ENOENT;
/* .. is obviously a directory */
vtyp = VDIR;
} else if (cnp->cn_namelen == 1 && *(cnp->cn_nameptr) == '.') {
} else if (is_dot) {
nid = VTOI(dvp);
/* . is obviously a directory */
vtyp = VDIR;
@ -1546,8 +1558,17 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
&vp);
*vpp = vp;
} else if (nid == VTOI(dvp)) {
vref(dvp);
*vpp = dvp;
if (is_dot) {
vref(dvp);
*vpp = dvp;
} else {
fuse_warn(fuse_get_mpdata(mp),
FSESS_WARN_ILLEGAL_INODE,
"Assigned same inode to both parent and "
"child.");
err = EIO;
}
} else {
struct fuse_vnode_data *fvdat;

41
tests/sys/fs/fusefs/create.cc
View File

@ -370,6 +370,47 @@ TEST_F(Create, ok)
leak(fd);
}
/*
* Nothing bad should happen if the server returns the parent's inode number
* for the newly created file. Regression test for bug 263662
* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=263662
*/
TEST_F(Create, parent_inode)
{
const char FULLPATH[] = "mountpoint/some_dir/some_file.txt";
const char RELDIRPATH[] = "some_dir";
const char RELPATH[] = "some_file.txt";
mode_t mode = 0755;
uint64_t ino = 42;
int fd;
expect_lookup(RELDIRPATH, ino, S_IFDIR | mode, 0, 1);
EXPECT_LOOKUP(ino, RELPATH)
.WillOnce(Invoke(ReturnErrno(ENOENT)));
expect_create(RELPATH, S_IFREG | mode,
ReturnImmediate([=](auto in __unused, auto& out) {
SET_OUT_HEADER_LEN(out, create);
out.body.create.entry.attr.mode = S_IFREG | mode;
/* Return the same inode as the parent dir */
out.body.create.entry.nodeid = ino;
out.body.create.entry.entry_valid = UINT64_MAX;
out.body.create.entry.attr_valid = UINT64_MAX;
}));
// FUSE_RELEASE 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_RELEASE);
}, Eq(true)),
_)
).Times(AtMost(1))
.WillOnce(Invoke([=](auto in __unused, auto &out __unused) { }));
fd = open(FULLPATH, O_CREAT | O_EXCL, mode);
ASSERT_EQ(-1, fd);
EXPECT_EQ(EIO, errno);
}
/*
* A regression test for a bug that affected old FUSE implementations:
* open(..., O_WRONLY | O_CREAT, 0444) should work despite the seeming

47
tests/sys/fs/fusefs/link.cc
View File

@ -176,6 +176,53 @@ TEST_F(Link, emlink)
EXPECT_EQ(EMLINK, errno);
}
/*
* A hard link should always have the same inode as its source. If it doesn't,
* then it's not a hard link.
*/
TEST_F(Link, bad_inode)
{
const char FULLPATH[] = "mountpoint/src";
const char RELPATH[] = "src";
const char FULLDST[] = "mountpoint/dst";
const char RELDST[] = "dst";
const uint64_t src_ino = 42;
const uint64_t dst_ino = 43;
mode_t mode = S_IFREG | 0644;
EXPECT_LOOKUP(FUSE_ROOT_ID, RELPATH)
.WillOnce(Invoke(ReturnErrno(ENOENT)));
EXPECT_LOOKUP(FUSE_ROOT_ID, RELDST)
.WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = mode;
out.body.entry.nodeid = dst_ino;
out.body.entry.attr.nlink = 1;
out.body.entry.attr_valid = UINT64_MAX;
out.body.entry.entry_valid = UINT64_MAX;
})));
EXPECT_CALL(*m_mock, process(
ResultOf([=](auto in) {
const char *name = (const char*)in.body.bytes
+ sizeof(struct fuse_link_in);
return (in.header.opcode == FUSE_LINK &&
in.body.link.oldnodeid == dst_ino &&
(0 == strcmp(name, RELPATH)));
}, Eq(true)),
_)
).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.nodeid = src_ino;
out.body.entry.attr.mode = mode;
out.body.entry.attr.nlink = 2;
out.body.entry.attr_valid = UINT64_MAX;
out.body.entry.entry_valid = UINT64_MAX;
})));
ASSERT_EQ(-1, link(FULLDST, FULLPATH));
ASSERT_EQ(EIO, errno);
}
TEST_F(Link, ok)
{
const char FULLPATH[] = "mountpoint/src";

31
tests/sys/fs/fusefs/lookup.cc
View File

@ -430,6 +430,37 @@ TEST_F(Lookup, ok)
ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno);
}
/*
* Lookup in a subdirectory of the fuse mount. The naughty server returns the
* same inode for the child as for the parent.
*/
TEST_F(Lookup, parent_inode)
{
const char FULLPATH[] = "mountpoint/some_dir/some_file.txt";
const char DIRPATH[] = "some_dir";
const char RELPATH[] = "some_file.txt";
uint64_t dir_ino = 2;
EXPECT_LOOKUP(FUSE_ROOT_ID, DIRPATH)
.WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
SET_OUT_HEADER_LEN(out, entry);
out.body.entry.attr.mode = S_IFDIR | 0755;
out.body.entry.nodeid = dir_ino;
})));
EXPECT_LOOKUP(dir_ino, 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 = dir_ino;
})));
/*
* access(2) is one of the few syscalls that will not (always) follow
* up a successful VOP_LOOKUP with another VOP.
*/
ASSERT_EQ(-1, access(FULLPATH, F_OK));
ASSERT_EQ(EIO, errno);
}
// Lookup in a subdirectory of the fuse mount
TEST_F(Lookup, subdir)
{

53
tests/sys/fs/fusefs/mkdir.cc
View File

@ -193,6 +193,59 @@ TEST_F(Mkdir, ok)
ASSERT_EQ(0, mkdir(FULLPATH, mode)) << strerror(errno);
}
/*
* Nothing bad should happen if the server returns the parent's inode number
* for the newly created directory. Regression test for bug 263662.
* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=263662
*/
TEST_F(Mkdir, parent_inode)
{
const char FULLPATH[] = "mountpoint/parent/some_dir";
const char PPATH[] = "parent";
const char RELPATH[] = "some_dir";
mode_t mode = 0755;
uint64_t ino = 42;
mode_t mask;
mask = umask(0);
(void)umask(mask);
expect_lookup(PPATH, ino, S_IFDIR | 0755, 0, 1);
EXPECT_LOOKUP(ino, RELPATH)
.WillOnce(Invoke(ReturnErrno(ENOENT)));
EXPECT_CALL(*m_mock, process(
ResultOf([=](auto in) {
const char *name = (const char*)in.body.bytes +
sizeof(fuse_mkdir_in);
return (in.header.opcode == FUSE_MKDIR &&
in.body.mkdir.mode == (S_IFDIR | mode) &&
in.body.mkdir.umask == mask &&
(0 == strcmp(RELPATH, name)));
}, Eq(true)),
_)
).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
SET_OUT_HEADER_LEN(out, entry);
out.body.create.entry.attr.mode = S_IFDIR | mode;
out.body.create.entry.nodeid = ino;
out.body.create.entry.entry_valid = UINT64_MAX;
out.body.create.entry.attr_valid = UINT64_MAX;
})));
// FUSE_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);
}, Eq(true)),
_)
).Times(AtMost(1))
.WillOnce(Invoke([=](auto in __unused, auto &out __unused) { }));
ASSERT_EQ(-1, mkdir(FULLPATH, mode));
ASSERT_EQ(EIO, errno);
usleep(100000);
}
TEST_F(Mkdir_7_8, ok)
{
const char FULLPATH[] = "mountpoint/some_dir";

32
tests/sys/fs/fusefs/mknod.cc
View File

@ -245,6 +245,38 @@ TEST_F(Mknod, socket)
leak(fd);
}
/*
* Nothing bad should happen if the server returns the parent's inode number
* for the newly created file. Regression test for bug 263662.
* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=263662
*/
TEST_F(Mknod, parent_inode)
{
const char FULLPATH[] = "mountpoint/parent/some_node";
const char PPATH[] = "parent";
const char RELPATH[] = "some_node";
mode_t mode = S_IFSOCK | 0755;
struct sockaddr_un sa;
int fd;
dev_t rdev = -1; /* Really it's a don't care */
uint64_t ino = 42;
expect_lookup(PPATH, ino, S_IFDIR | 0755, 0, 1);
EXPECT_LOOKUP(ino, RELPATH)
.WillOnce(Invoke(ReturnErrno(ENOENT)));
expect_mknod(ino, RELPATH, ino, mode, rdev);
fd = socket(AF_UNIX, SOCK_STREAM, 0);
ASSERT_LE(0, fd) << strerror(errno);
sa.sun_family = AF_UNIX;
strlcpy(sa.sun_path, FULLPATH, sizeof(sa.sun_path));
sa.sun_len = sizeof(FULLPATH);
ASSERT_EQ(-1, bind(fd, (struct sockaddr*)&sa, sizeof(sa)));
ASSERT_EQ(EIO, errno);
leak(fd);
}
/*
* fusefs(5) lacks VOP_WHITEOUT support. No bugzilla entry, because that's a
* feature, not a bug

22
tests/sys/fs/fusefs/symlink.cc
View File

@ -165,6 +165,28 @@ TEST_F(Symlink, ok)
EXPECT_EQ(0, symlink(dst, FULLPATH)) << strerror(errno);
}
/*
* Nothing bad should happen if the server returns the parent's inode number
* for the newly created symlink. Regression test for bug 263662.
* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=263662
*/
TEST_F(Symlink, parent_ino)
{
const char FULLPATH[] = "mountpoint/parent/src";
const char PPATH[] = "parent";
const char RELPATH[] = "src";
const char dst[] = "dst";
const uint64_t ino = 42;
expect_lookup(PPATH, ino, S_IFDIR | 0755, 0, 1);
EXPECT_LOOKUP(ino, RELPATH)
.WillOnce(Invoke(ReturnErrno(ENOENT)));
expect_symlink(ino, dst, RELPATH);
EXPECT_EQ(-1, symlink(dst, FULLPATH));
EXPECT_EQ(EIO, errno);
}
TEST_F(Symlink_7_8, ok)
{
const char FULLPATH[] = "mountpoint/src";