svn commit: r347189 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs
Alan Somers
asomers at FreeBSD.org
Mon May 6 16:17:58 UTC 2019
Author: asomers
Date: Mon May 6 16:17:55 2019
New Revision: 347189
URL: https://svnweb.freebsd.org/changeset/base/347189
Log:
fusefs: clear SUID & SGID after a successful write by a non-owner
Reported by: pjdfstest
Sponsored by: The FreeBSD Foundation
Modified:
projects/fuse2/sys/fs/fuse/fuse_internal.c
projects/fuse2/sys/fs/fuse/fuse_internal.h
projects/fuse2/sys/fs/fuse/fuse_io.c
projects/fuse2/sys/fs/fuse/fuse_vnops.c
projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc
projects/fuse2/tests/sys/fs/fusefs/default_permissions_privileged.cc
Modified: projects/fuse2/sys/fs/fuse/fuse_internal.c
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_internal.c Mon May 6 16:17:38 2019 (r347188)
+++ projects/fuse2/sys/fs/fuse/fuse_internal.c Mon May 6 16:17:55 2019 (r347189)
@@ -740,6 +740,114 @@ fuse_internal_send_init(struct fuse_data *data, struct
fdisp_destroy(&fdi);
}
+/*
+ * Send a FUSE_SETATTR operation with no permissions checks. If cred is NULL,
+ * send the request with root credentials
+ */
+int fuse_internal_setattr(struct vnode *vp, struct vattr *vap,
+ struct thread *td, struct ucred *cred)
+{
+ struct fuse_dispatcher fdi;
+ struct fuse_setattr_in *fsai;
+ struct mount *mp;
+ pid_t pid = td->td_proc->p_pid;
+ struct fuse_data *data;
+ int dataflags;
+ int err = 0;
+ enum vtype vtyp;
+ int sizechanged = -1;
+ uint64_t newsize = 0;
+
+ mp = vnode_mount(vp);
+ data = fuse_get_mpdata(mp);
+ dataflags = data->dataflags;
+
+ fdisp_init(&fdi, sizeof(*fsai));
+ fdisp_make_vp(&fdi, FUSE_SETATTR, vp, td, cred);
+ if (!cred) {
+ fdi.finh->uid = 0;
+ fdi.finh->gid = 0;
+ }
+ fsai = fdi.indata;
+ fsai->valid = 0;
+
+ if (vap->va_uid != (uid_t)VNOVAL) {
+ fsai->uid = vap->va_uid;
+ fsai->valid |= FATTR_UID;
+ }
+ if (vap->va_gid != (gid_t)VNOVAL) {
+ fsai->gid = vap->va_gid;
+ fsai->valid |= FATTR_GID;
+ }
+ if (vap->va_size != VNOVAL) {
+ struct fuse_filehandle *fufh = NULL;
+
+ /*Truncate to a new value. */
+ fsai->size = vap->va_size;
+ sizechanged = 1;
+ newsize = vap->va_size;
+ fsai->valid |= FATTR_SIZE;
+
+ fuse_filehandle_getrw(vp, FWRITE, &fufh, cred, pid);
+ if (fufh) {
+ fsai->fh = fufh->fh_id;
+ fsai->valid |= FATTR_FH;
+ }
+ }
+ if (vap->va_atime.tv_sec != VNOVAL) {
+ fsai->atime = vap->va_atime.tv_sec;
+ fsai->atimensec = vap->va_atime.tv_nsec;
+ fsai->valid |= FATTR_ATIME;
+ }
+ if (vap->va_mtime.tv_sec != VNOVAL) {
+ fsai->mtime = vap->va_mtime.tv_sec;
+ fsai->mtimensec = vap->va_mtime.tv_nsec;
+ fsai->valid |= FATTR_MTIME;
+ }
+ if (vap->va_mode != (mode_t)VNOVAL) {
+ fsai->mode = vap->va_mode & ALLPERMS;
+ fsai->valid |= FATTR_MODE;
+ }
+ if (!fsai->valid) {
+ goto out;
+ }
+
+ if ((err = fdisp_wait_answ(&fdi)))
+ goto out;
+ vtyp = IFTOVT(((struct fuse_attr_out *)fdi.answ)->attr.mode);
+
+ if (vnode_vtype(vp) != vtyp) {
+ if (vnode_vtype(vp) == VNON && vtyp != VNON) {
+ SDT_PROBE2(fusefs, , internal, trace, 1, "FUSE: Dang! "
+ "vnode_vtype is VNON and vtype isn't.");
+ } else {
+ /*
+ * STALE vnode, ditch
+ *
+ * The vnode has changed its type "behind our back".
+ * There's nothing really we can do, so let us just
+ * force an internal revocation and tell the caller to
+ * try again, if interested.
+ */
+ fuse_internal_vnode_disappear(vp);
+ err = EAGAIN;
+ }
+ }
+ if (err == 0) {
+ struct fuse_attr_out *fao = (struct fuse_attr_out*)fdi.answ;
+ fuse_internal_cache_attrs(vp, &fao->attr, fao->attr_valid,
+ fao->attr_valid_nsec, NULL);
+ }
+
+out:
+ fdisp_destroy(&fdi);
+ if (!err && sizechanged) {
+ fuse_vnode_setsize(vp, cred, newsize);
+ VTOFUD(vp)->flag &= ~FN_SIZECHANGE;
+ }
+ return err;
+}
+
#ifdef ZERO_PAD_INCOMPLETE_BUFS
static int
isbzero(void *buf, size_t len)
Modified: projects/fuse2/sys/fs/fuse/fuse_internal.h
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_internal.h Mon May 6 16:17:38 2019 (r347188)
+++ projects/fuse2/sys/fs/fuse/fuse_internal.h Mon May 6 16:17:55 2019 (r347189)
@@ -247,6 +247,10 @@ int fuse_internal_rename(struct vnode *fdvp, struct co
void fuse_internal_vnode_disappear(struct vnode *vp);
+/* setattr */
+int fuse_internal_setattr(struct vnode *vp, struct vattr *va,
+ struct thread *td, struct ucred *cred);
+
/* strategy */
/* entity creation */
Modified: projects/fuse2/sys/fs/fuse/fuse_io.c
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_io.c Mon May 6 16:17:38 2019 (r347188)
+++ projects/fuse2/sys/fs/fuse/fuse_io.c Mon May 6 16:17:55 2019 (r347189)
@@ -72,6 +72,7 @@ __FBSDID("$FreeBSD$");
#include <sys/sx.h>
#include <sys/mutex.h>
#include <sys/rwlock.h>
+#include <sys/priv.h>
#include <sys/proc.h>
#include <sys/mount.h>
#include <sys/vnode.h>
@@ -106,6 +107,9 @@ SDT_PROVIDER_DECLARE(fusefs);
*/
SDT_PROBE_DEFINE2(fusefs, , io, trace, "int", "char*");
+static void
+fuse_io_clear_suid_on_write(struct vnode *vp, struct ucred *cred,
+ struct thread *td);
static int
fuse_read_directbackend(struct vnode *vp, struct uio *uio,
struct ucred *cred, struct fuse_filehandle *fufh);
@@ -119,6 +123,43 @@ static int
fuse_write_biobackend(struct vnode *vp, struct uio *uio,
struct ucred *cred, struct fuse_filehandle *fufh, int ioflag, pid_t pid);
+/*
+ * FreeBSD clears the SUID and SGID bits on any write by a non-root user.
+ */
+static void
+fuse_io_clear_suid_on_write(struct vnode *vp, struct ucred *cred,
+ struct thread *td)
+{
+ struct fuse_data *data;
+ struct mount *mp;
+ struct vattr va;
+ int dataflags;
+
+ mp = vnode_mount(vp);
+ data = fuse_get_mpdata(mp);
+ dataflags = data->dataflags;
+
+ if (dataflags & FSESS_DEFAULT_PERMISSIONS) {
+ if (priv_check_cred(cred, PRIV_VFS_RETAINSUGID)) {
+ fuse_internal_getattr(vp, &va, cred, td);
+ if (va.va_mode & (S_ISUID | S_ISGID)) {
+ mode_t mode = va.va_mode & ~(S_ISUID | S_ISGID);
+ /* Clear all vattr fields except mode */
+ vattr_null(&va);
+ va.va_mode = mode;
+
+ /*
+ * Ignore fuse_internal_setattr's return value,
+ * because at this point the write operation has
+ * already succeeded and we don't want to return
+ * failing status for that.
+ */
+ (void)fuse_internal_setattr(vp, &va, td, NULL);
+ }
+ }
+ }
+}
+
SDT_PROBE_DEFINE5(fusefs, , io, io_dispatch, "struct vnode*", "struct uio*",
"int", "struct ucred*", "struct fuse_filehandle*");
int
@@ -194,6 +235,7 @@ fuse_io_dispatch(struct vnode *vp, struct uio *uio, in
err = fuse_write_biobackend(vp, uio, cred, fufh, ioflag,
pid);
}
+ fuse_io_clear_suid_on_write(vp, cred, uio->uio_td);
break;
default:
panic("uninterpreted mode passed to fuse_io_dispatch");
@@ -810,6 +852,9 @@ fuse_io_strategy(struct vnode *vp, struct buf *bp)
bp->b_ioflags |= BIO_ERROR;
bp->b_flags |= B_INVAL;
bp->b_error = error;
+ } else {
+ fuse_io_clear_suid_on_write(vp, cred,
+ uio.uio_td);
}
bp->b_dirtyoff = bp->b_dirtyend = 0;
}
Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_vnops.c Mon May 6 16:17:38 2019 (r347188)
+++ projects/fuse2/sys/fs/fuse/fuse_vnops.c Mon May 6 16:17:55 2019 (r347189)
@@ -1516,44 +1516,33 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
struct vattr *vap = ap->a_vap;
struct ucred *cred = ap->a_cred;
struct thread *td = curthread;
- struct fuse_dispatcher fdi;
- struct fuse_setattr_in *fsai;
struct mount *mp;
- pid_t pid = td->td_proc->p_pid;
struct fuse_data *data;
int dataflags;
int err = 0;
- enum vtype vtyp;
- int sizechanged = 0;
- uint64_t newsize = 0;
accmode_t accmode = 0;
+ bool checkperm;
mp = vnode_mount(vp);
data = fuse_get_mpdata(mp);
dataflags = data->dataflags;
+ checkperm = dataflags & FSESS_DEFAULT_PERMISSIONS;
if (fuse_isdeadfs(vp)) {
return ENXIO;
}
- fdisp_init(&fdi, sizeof(*fsai));
- fdisp_make_vp(&fdi, FUSE_SETATTR, vp, td, cred);
- fsai = fdi.indata;
- fsai->valid = 0;
if (vap->va_uid != (uid_t)VNOVAL) {
- if (dataflags & FSESS_DEFAULT_PERMISSIONS) {
+ if (checkperm) {
/* Only root may change a file's owner */
err = priv_check_cred(cred, PRIV_VFS_CHOWN);
if (err)
- goto out;
+ return err;;
}
- fsai->uid = vap->va_uid;
- fsai->valid |= FATTR_UID;
accmode |= VADMIN;
}
if (vap->va_gid != (gid_t)VNOVAL) {
- if (dataflags & FSESS_DEFAULT_PERMISSIONS &&
- !groupmember(vap->va_gid, cred))
+ if (checkperm && !groupmember(vap->va_gid, cred))
{
/*
* Non-root users may only chgrp to one of their own
@@ -1561,112 +1550,56 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
*/
err = priv_check_cred(cred, PRIV_VFS_CHOWN);
if (err)
- goto out;
+ return err;
}
- fsai->gid = vap->va_gid;
- fsai->valid |= FATTR_GID;
accmode |= VADMIN;
}
if (vap->va_size != VNOVAL) {
-
- struct fuse_filehandle *fufh = NULL;
-
- /*Truncate to a new value. */
- fsai->size = vap->va_size;
- sizechanged = 1;
- newsize = vap->va_size;
- fsai->valid |= FATTR_SIZE;
- accmode |= VWRITE;
-
- fuse_filehandle_getrw(vp, FWRITE, &fufh, cred, pid);
- if (fufh) {
- fsai->fh = fufh->fh_id;
- fsai->valid |= FATTR_FH;
+ switch (vp->v_type) {
+ case VDIR:
+ return (EISDIR);
+ case VLNK:
+ case VREG:
+ if (vfs_isrdonly(mp))
+ return (EROFS);
+ break;
+ default:
+ /*
+ * According to POSIX, the result is unspecified
+ * for file types other than regular files,
+ * directories and shared memory objects. We
+ * don't support shared memory objects in the file
+ * system, and have dubious support for truncating
+ * symlinks. Just ignore the request in other cases.
+ */
+ return (0);
}
+ accmode |= VWRITE;
}
- if (vap->va_atime.tv_sec != VNOVAL) {
- fsai->atime = vap->va_atime.tv_sec;
- fsai->atimensec = vap->va_atime.tv_nsec;
- fsai->valid |= FATTR_ATIME;
+ /*
+ * TODO: for atime and mtime, only require VWRITE if UTIMENS_NULL is
+ * set. PR 237181
+ */
+ if (vap->va_atime.tv_sec != VNOVAL)
accmode |= VADMIN;
- /*
- * TODO: only require VWRITE if UTIMENS_NULL is set. PR 237181
- */
- }
- if (vap->va_mtime.tv_sec != VNOVAL) {
- fsai->mtime = vap->va_mtime.tv_sec;
- fsai->mtimensec = vap->va_mtime.tv_nsec;
- fsai->valid |= FATTR_MTIME;
+ if (vap->va_mtime.tv_sec != VNOVAL)
accmode |= VADMIN;
- /*
- * TODO: only require VWRITE if UTIMENS_NULL is set. PR 237181
- */
- }
if (vap->va_mode != (mode_t)VNOVAL) {
/* Only root may set the sticky bit on non-directories */
- if (dataflags & FSESS_DEFAULT_PERMISSIONS &&
- vp->v_type != VDIR && (vap->va_mode & S_ISTXT))
- {
- if (priv_check_cred(cred, PRIV_VFS_STICKYFILE)) {
- err = EFTYPE;
- goto out;
- }
- }
- fsai->mode = vap->va_mode & ALLPERMS;
- fsai->valid |= FATTR_MODE;
+ if (checkperm && vp->v_type != VDIR && (vap->va_mode & S_ISTXT)
+ && priv_check_cred(cred, PRIV_VFS_STICKYFILE))
+ return EFTYPE;
accmode |= VADMIN;
}
- if (!fsai->valid) {
- goto out;
- }
- vtyp = vnode_vtype(vp);
- if (fsai->valid & FATTR_SIZE && vtyp == VDIR) {
- err = EISDIR;
- goto out;
- }
- if (vfs_isrdonly(vnode_mount(vp))) {
- err = EROFS;
- goto out;
- }
+ if (vfs_isrdonly(mp))
+ return EROFS;
+
err = fuse_internal_access(vp, accmode, td, cred);
if (err)
- goto out;
-
- if ((err = fdisp_wait_answ(&fdi)))
- goto out;
- vtyp = IFTOVT(((struct fuse_attr_out *)fdi.answ)->attr.mode);
-
- if (vnode_vtype(vp) != vtyp) {
- if (vnode_vtype(vp) == VNON && vtyp != VNON) {
- SDT_PROBE2(fusefs, , vnops, trace, 1, "FUSE: Dang! "
- "vnode_vtype is VNON and vtype isn't.");
- } else {
- /*
- * STALE vnode, ditch
- *
- * The vnode has changed its type "behind our back".
- * There's nothing really we can do, so let us just
- * force an internal revocation and tell the caller to
- * try again, if interested.
- */
- fuse_internal_vnode_disappear(vp);
- err = EAGAIN;
- }
- }
- if (err == 0) {
- struct fuse_attr_out *fao = (struct fuse_attr_out*)fdi.answ;
- fuse_internal_cache_attrs(vp, &fao->attr, fao->attr_valid,
- fao->attr_valid_nsec, NULL);
- }
-
-out:
- fdisp_destroy(&fdi);
- if (!err && sizechanged) {
- fuse_vnode_setsize(vp, cred, newsize);
- VTOFUD(vp)->flag &= ~FN_SIZECHANGE;
- }
- return err;
+ return err;
+ else
+ return fuse_internal_setattr(vp, vap, td, cred);
}
/*
Modified: projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc
==============================================================================
--- projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc Mon May 6 16:17:38 2019 (r347188)
+++ projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc Mon May 6 16:17:55 2019 (r347189)
@@ -68,6 +68,24 @@ virtual void SetUp() {
}
public:
+void expect_chmod(uint64_t ino, mode_t mode)
+{
+ EXPECT_CALL(*m_mock, process(
+ ResultOf([=](auto in) {
+ return (in->header.opcode == FUSE_SETATTR &&
+ in->header.nodeid == ino &&
+ in->body.setattr.valid == FATTR_MODE &&
+ in->body.setattr.mode == mode);
+ }, Eq(true)),
+ _)
+ ).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) {
+ SET_OUT_HEADER_LEN(out, attr);
+ out->body.attr.attr.ino = ino; // Must match nodeid
+ out->body.attr.attr.mode = S_IFREG | mode;
+ out->body.attr.attr_valid = UINT64_MAX;
+ })));
+}
+
void expect_getattr(uint64_t ino, mode_t mode, uint64_t attr_valid, int times,
uid_t uid = 0, gid_t gid = 0)
{
@@ -104,6 +122,7 @@ class Lookup: public DefaultPermissions {};
class Open: public DefaultPermissions {};
class Setattr: public DefaultPermissions {};
class Unlink: public DefaultPermissions {};
+class Write: public DefaultPermissions {};
/*
* Test permission handling during create, mkdir, mknod, link, symlink, and
@@ -934,4 +953,56 @@ TEST_F(Unlink, sticky_directory)
ASSERT_EQ(-1, unlink(FULLPATH));
ASSERT_EQ(EPERM, errno);
+}
+
+/* A write by a non-owner should clear a file's SUID bit */
+TEST_F(Write, clear_suid)
+{
+ const char FULLPATH[] = "mountpoint/some_file.txt";
+ const char RELPATH[] = "some_file.txt";
+ struct stat sb;
+ uint64_t ino = 42;
+ mode_t oldmode = 04777;
+ mode_t newmode = 0777;
+ char wbuf[1] = {'x'};
+ int fd;
+
+ expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+ expect_lookup(RELPATH, ino, S_IFREG | oldmode, UINT64_MAX);
+ expect_open(ino, 0, 1);
+ expect_write(ino, 0, sizeof(wbuf), sizeof(wbuf), 0, wbuf);
+ expect_chmod(ino, newmode);
+
+ fd = open(FULLPATH, O_WRONLY);
+ ASSERT_LE(0, fd) << strerror(errno);
+ ASSERT_EQ(1, write(fd, wbuf, sizeof(wbuf))) << strerror(errno);
+ ASSERT_EQ(0, fstat(fd, &sb)) << strerror(errno);
+ EXPECT_EQ(S_IFREG | newmode, sb.st_mode);
+ /* Deliberately leak fd. close(2) will be tested in release.cc */
+}
+
+/* A write by a non-owner should clear a file's SGID bit */
+TEST_F(Write, clear_sgid)
+{
+ const char FULLPATH[] = "mountpoint/some_file.txt";
+ const char RELPATH[] = "some_file.txt";
+ struct stat sb;
+ uint64_t ino = 42;
+ mode_t oldmode = 02777;
+ mode_t newmode = 0777;
+ char wbuf[1] = {'x'};
+ int fd;
+
+ expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+ expect_lookup(RELPATH, ino, S_IFREG | oldmode, UINT64_MAX);
+ expect_open(ino, 0, 1);
+ expect_write(ino, 0, sizeof(wbuf), sizeof(wbuf), 0, wbuf);
+ expect_chmod(ino, newmode);
+
+ fd = open(FULLPATH, O_WRONLY);
+ ASSERT_LE(0, fd) << strerror(errno);
+ ASSERT_EQ(1, write(fd, wbuf, sizeof(wbuf))) << strerror(errno);
+ ASSERT_EQ(0, fstat(fd, &sb)) << strerror(errno);
+ EXPECT_EQ(S_IFREG | newmode, sb.st_mode);
+ /* Deliberately leak fd. close(2) will be tested in release.cc */
}
Modified: projects/fuse2/tests/sys/fs/fusefs/default_permissions_privileged.cc
==============================================================================
--- projects/fuse2/tests/sys/fs/fusefs/default_permissions_privileged.cc Mon May 6 16:17:38 2019 (r347188)
+++ projects/fuse2/tests/sys/fs/fusefs/default_permissions_privileged.cc Mon May 6 16:17:55 2019 (r347189)
@@ -98,7 +98,7 @@ void expect_lookup(const char *relpath, uint64_t ino,
class Setattr: public DefaultPermissionsPrivileged {};
-TEST_F(Setattr, sticky_regular_file_eftype)
+TEST_F(Setattr, sticky_regular_file)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";
More information about the svn-src-projects
mailing list