svn commit: r346101 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs
Alan Somers
asomers at FreeBSD.org
Tue Sep 3 14:06:40 UTC 2019
Author: asomers
Date: Wed Apr 10 21:10:21 2019
New Revision: 346101
URL: https://svnweb.freebsd.org/changeset/base/346101
Log:
fusefs: various cleanups
* Eliminate fuse_access_param. Whatever it was supposed to do, it seems
like it was never complete. The only real function it ever seems to have
had was a minor performance optimization, which I've already eliminated.
* Make extended attribute operations obey the allow_other mount option.
* Allow unprivileged access to the SYSTEM extattr namespace when
-o default_permissions is not in use.
* Disallow setextattr and deleteextattr on read-only mounts.
* Add tests for a few more error cases.
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_vnops.c
projects/fuse2/tests/sys/fs/fusefs/allow_other.cc
projects/fuse2/tests/sys/fs/fusefs/lookup.cc
projects/fuse2/tests/sys/fs/fusefs/setattr.cc
projects/fuse2/tests/sys/fs/fusefs/xattr.cc
Modified: projects/fuse2/sys/fs/fuse/fuse_internal.c
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_internal.c Wed Apr 10 20:44:54 2019 (r346100)
+++ projects/fuse2/sys/fs/fuse/fuse_internal.c Wed Apr 10 21:10:21 2019 (r346101)
@@ -110,7 +110,6 @@ static int isbzero(void *buf, size_t len);
int
fuse_internal_access(struct vnode *vp,
accmode_t mode,
- struct fuse_access_param *facp,
struct thread *td,
struct ucred *cred)
{
@@ -143,13 +142,9 @@ fuse_internal_access(struct vnode *vp,
}
/* Unless explicitly permitted, deny everyone except the fs owner. */
- if (!(facp->facc_flags)) {
- if (!(dataflags & FSESS_DAEMON_CAN_SPY)) {
- int denied = fuse_match_cred(data->daemoncred, cred);
-
- if (denied)
- return EPERM;
- }
+ if (!(dataflags & FSESS_DAEMON_CAN_SPY)) {
+ if (fuse_match_cred(data->daemoncred, cred))
+ return EPERM;
}
if (dataflags & FSESS_DEFAULT_PERMISSIONS) {
Modified: projects/fuse2/sys/fs/fuse/fuse_internal.h
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_internal.h Wed Apr 10 20:44:54 2019 (r346100)
+++ projects/fuse2/sys/fs/fuse/fuse_internal.h Wed Apr 10 21:10:21 2019 (r346101)
@@ -197,27 +197,6 @@ fuse_validity_2_timespec(const struct fuse_entry_out *
/* access */
-
-#define FVP_ACCESS_NOOP 0x01
-
-#define FACCESS_VA_VALID 0x01
-/*
- * Caller must be the directory's owner, or the superuser, or the sticky bit
- * must not be set
- */
-#define FACCESS_STICKY 0x04
-/* Caller requires access to change file's owner */
-#define FACCESS_CHOWN 0x08
-#define FACCESS_SETGID 0x12
-
-#define FACCESS_XQUERIES (FACCESS_STICKY | FACCESS_CHOWN | FACCESS_SETGID)
-
-struct fuse_access_param {
- uid_t xuid;
- gid_t xgid;
- uint32_t facc_flags;
-};
-
static inline int
fuse_match_cred(struct ucred *basecred, struct ucred *usercred)
{
@@ -233,7 +212,7 @@ fuse_match_cred(struct ucred *basecred, struct ucred *
}
int fuse_internal_access(struct vnode *vp, accmode_t mode,
- struct fuse_access_param *facp, struct thread *td, struct ucred *cred);
+ struct thread *td, struct ucred *cred);
/* attributes */
void fuse_internal_cache_attrs(struct vnode *vp, struct fuse_attr *attr,
Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_vnops.c Wed Apr 10 20:44:54 2019 (r346100)
+++ projects/fuse2/sys/fs/fuse/fuse_vnops.c Wed Apr 10 21:10:21 2019 (r346101)
@@ -211,6 +211,37 @@ uma_zone_t fuse_pbuf_zone;
#define fuse_vm_page_lock_queues() ((void)0)
#define fuse_vm_page_unlock_queues() ((void)0)
+/* Check permission for extattr operations, much like extattr_check_cred */
+static int
+fuse_extattr_check_cred(struct vnode *vp, int ns, struct ucred *cred,
+ struct thread *td, accmode_t accmode)
+{
+ struct mount *mp = vnode_mount(vp);
+ struct fuse_data *data = fuse_get_mpdata(mp);
+
+ /*
+ * Kernel-invoked always succeeds.
+ */
+ if (cred == NOCRED)
+ return (0);
+
+ /*
+ * Do not allow privileged processes in jail to directly manipulate
+ * system attributes.
+ */
+ switch (ns) {
+ case EXTATTR_NAMESPACE_SYSTEM:
+ if (data->dataflags & FSESS_DEFAULT_PERMISSIONS) {
+ return (priv_check_cred(cred, PRIV_VFS_EXTATTR_SYSTEM));
+ }
+ /* FALLTHROUGH */
+ case EXTATTR_NAMESPACE_USER:
+ return (fuse_internal_access(vp, accmode, td, cred));
+ default:
+ return (EPERM);
+ }
+}
+
/* Get a filehandle for a directory */
static int
fuse_filehandle_get_dir(struct vnode *vp, struct fuse_filehandle **fufhp,
@@ -272,7 +303,6 @@ fuse_vnop_access(struct vop_access_args *ap)
int accmode = ap->a_accmode;
struct ucred *cred = ap->a_cred;
- struct fuse_access_param facp;
struct fuse_data *data = fuse_get_mpdata(vnode_mount(vp));
int err;
@@ -295,9 +325,8 @@ fuse_vnop_access(struct vop_access_args *ap)
if (vnode_islnk(vp)) {
return 0;
}
- bzero(&facp, sizeof(facp));
- err = fuse_internal_access(vp, accmode, &facp, ap->a_td, ap->a_cred);
+ err = fuse_internal_access(vp, accmode, ap->a_td, ap->a_cred);
return err;
}
@@ -711,7 +740,6 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
struct fuse_attr *fattr = NULL;
uint64_t nid;
- struct fuse_access_param facp;
if (fuse_isdeadfs(dvp)) {
*vpp = NULL;
@@ -730,9 +758,9 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
* See further comments at further access checks.
*/
- bzero(&facp, sizeof(facp));
+ /* TODO: consider eliminating this. Is there any good reason for it? */
if (vnode_isvroot(dvp)) { /* early permission check hack */
- if ((err = fuse_internal_access(dvp, VEXEC, &facp, td, cred))) {
+ if ((err = fuse_internal_access(dvp, VEXEC, td, cred))) {
return err;
}
}
@@ -831,8 +859,7 @@ calldaemon:
if (lookup_err) {
/* Entry not found */
if ((nameiop == CREATE || nameiop == RENAME) && islastcn) {
- err = fuse_internal_access(dvp, VWRITE, &facp, td,
- cred);
+ err = fuse_internal_access(dvp, VWRITE, td, cred);
if (err)
goto out;
@@ -861,11 +888,8 @@ calldaemon:
fattr = &(feo->attr);
}
- /* TODO: check for ENOTDIR */
-
if ((nameiop == DELETE || nameiop == RENAME) && islastcn) {
- err = fuse_internal_access(dvp, VWRITE, &facp,
- td, cred);
+ err = fuse_internal_access(dvp, VWRITE, td, cred);
if (err != 0)
goto out;
/*
@@ -878,10 +902,8 @@ calldaemon:
struct vattr dvattr;
fuse_internal_getattr(dvp, &dvattr, cred, td);
if ((dvattr.va_mode & S_ISTXT) &&
- fuse_internal_access(dvp, VADMIN, &facp,
- cnp->cn_thread, cnp->cn_cred) &&
- fuse_internal_access(*vpp, VADMIN, &facp,
- cnp->cn_thread, cnp->cn_cred)) {
+ fuse_internal_access(dvp, VADMIN, td, cred) &&
+ fuse_internal_access(*vpp, VADMIN, td, cred)) {
err = EPERM;
goto out;
}
@@ -933,10 +955,6 @@ calldaemon:
if (err) {
goto out;
}
- /*err = fuse_lookup_check(dvp, vp, &facp, td, cred,*/
- /*nameiop, islastcn);*/
- /*if (err)*/
- /*goto out;*/
*vpp = vp;
/*
* Save the name for use in VOP_RENAME later.
@@ -1088,15 +1106,12 @@ out:
*/
int tmpvtype = vnode_vtype(*vpp);
- bzero(&facp, sizeof(facp));
- /*the early perm check hack */
- facp.facc_flags |= FACCESS_VA_VALID;
-
if ((tmpvtype != VDIR) && (tmpvtype != VLNK)) {
err = ENOTDIR;
}
if (!err && !vnode_mountedhere(*vpp)) {
- err = fuse_internal_access(*vpp, VEXEC, &facp, td, cred);
+ err = fuse_internal_access(*vpp, VEXEC,
+ td, cred);
}
if (err) {
if (tmpvtype == VLNK)
@@ -1528,7 +1543,6 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
struct thread *td = curthread;
struct fuse_dispatcher fdi;
struct fuse_setattr_in *fsai;
- struct fuse_access_param facp;
pid_t pid = td->td_proc->p_pid;
int err = 0;
@@ -1545,19 +1559,12 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
fsai = fdi.indata;
fsai->valid = 0;
- bzero(&facp, sizeof(facp));
-
- facp.xuid = vap->va_uid;
- facp.xgid = vap->va_gid;
-
if (vap->va_uid != (uid_t)VNOVAL) {
- facp.facc_flags |= FACCESS_CHOWN;
fsai->uid = vap->va_uid;
fsai->valid |= FATTR_UID;
accmode |= VADMIN;
}
if (vap->va_gid != (gid_t)VNOVAL) {
- facp.facc_flags |= FACCESS_CHOWN;
fsai->gid = vap->va_gid;
fsai->valid |= FATTR_GID;
accmode |= VADMIN;
@@ -1615,7 +1622,7 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
err = EROFS;
goto out;
}
- err = fuse_internal_access(vp, accmode, &facp, td, cred);
+ err = fuse_internal_access(vp, accmode, td, cred);
if (err)
goto out;
@@ -2028,7 +2035,7 @@ fuse_vnop_getextattr(struct vop_getextattr_args *ap)
if (fuse_isdeadfs(vp))
return (ENXIO);
- err = extattr_check_cred(vp, ap->a_attrnamespace, cred, td, VREAD);
+ err = fuse_extattr_check_cred(vp, ap->a_attrnamespace, cred, td, VREAD);
if (err)
return err;
@@ -2109,11 +2116,15 @@ fuse_vnop_setextattr(struct vop_setextattr_args *ap)
if (fuse_isdeadfs(vp))
return (ENXIO);
+ if (vfs_isrdonly(mp))
+ return EROFS;
+
/* Deleting xattrs must use VOP_DELETEEXTATTR instead */
if (ap->a_uio == NULL)
return (EINVAL);
- err = extattr_check_cred(vp, ap->a_attrnamespace, cred, td, VWRITE);
+ err = fuse_extattr_check_cred(vp, ap->a_attrnamespace, cred, td,
+ VWRITE);
if (err)
return err;
@@ -2244,7 +2255,7 @@ fuse_vnop_listextattr(struct vop_listextattr_args *ap)
if (fuse_isdeadfs(vp))
return (ENXIO);
- err = extattr_check_cred(vp, ap->a_attrnamespace, cred, td, VREAD);
+ err = fuse_extattr_check_cred(vp, ap->a_attrnamespace, cred, td, VREAD);
if (err)
return err;
@@ -2352,7 +2363,11 @@ fuse_vnop_deleteextattr(struct vop_deleteextattr_args
if (fuse_isdeadfs(vp))
return (ENXIO);
- err = extattr_check_cred(vp, ap->a_attrnamespace, cred, td, VWRITE);
+ if (vfs_isrdonly(mp))
+ return EROFS;
+
+ err = fuse_extattr_check_cred(vp, ap->a_attrnamespace, cred, td,
+ VWRITE);
if (err)
return err;
Modified: projects/fuse2/tests/sys/fs/fusefs/allow_other.cc
==============================================================================
--- projects/fuse2/tests/sys/fs/fusefs/allow_other.cc Wed Apr 10 20:44:54 2019 (r346100)
+++ projects/fuse2/tests/sys/fs/fusefs/allow_other.cc Wed Apr 10 21:10:21 2019 (r346101)
@@ -35,6 +35,7 @@
extern "C" {
#include <sys/types.h>
+#include <sys/extattr.h>
#include <fcntl.h>
#include <unistd.h>
}
@@ -212,6 +213,52 @@ TEST_F(NoAllowOther, disallowed_beneath_root)
fd = openat(dfd, RELPATH2, O_RDONLY);
if (fd >= 0) {
fprintf(stderr, "openat should've failed\n");
+ return(1);
+ } else if (errno != EPERM) {
+ fprintf(stderr, "Unexpected error: %s\n",
+ strerror(errno));
+ return(1);
+ }
+ return 0;
+ }
+ );
+}
+
+/*
+ * Provide coverage for the extattr methods, which have a slightly different
+ * code path
+ */
+TEST_F(NoAllowOther, setextattr)
+{
+ int ino = 42;
+
+ fork(true, [&] {
+ EXPECT_LOOKUP(1, RELPATH)
+ .WillOnce(Invoke(
+ ReturnImmediate([=](auto in __unused, auto out) {
+ SET_OUT_HEADER_LEN(out, entry);
+ out->body.entry.attr_valid = UINT64_MAX;
+ out->body.entry.entry_valid = UINT64_MAX;
+ out->body.entry.attr.mode = S_IFREG | 0644;
+ out->body.entry.nodeid = ino;
+ })));
+
+ /*
+ * lookup the file to get it into the cache.
+ * Otherwise, the unprivileged lookup will fail with
+ * EACCES
+ */
+ ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno);
+ }, [&]() {
+ const char value[] = "whatever";
+ ssize_t value_len = strlen(value) + 1;
+ int ns = EXTATTR_NAMESPACE_USER;
+ ssize_t r;
+
+ r = extattr_set_file(FULLPATH, ns, "foo",
+ (void*)value, value_len);
+ if (r >= 0) {
+ fprintf(stderr, "should've failed\n");
return(1);
} else if (errno != EPERM) {
fprintf(stderr, "Unexpected error: %s\n",
Modified: projects/fuse2/tests/sys/fs/fusefs/lookup.cc
==============================================================================
--- projects/fuse2/tests/sys/fs/fusefs/lookup.cc Wed Apr 10 20:44:54 2019 (r346100)
+++ projects/fuse2/tests/sys/fs/fusefs/lookup.cc Wed Apr 10 21:10:21 2019 (r346101)
@@ -144,8 +144,23 @@ TEST_F(Lookup, enoent)
EXPECT_EQ(ENOENT, errno);
}
-//TODO: test ENOTDIR
+TEST_F(Lookup, enotdir)
+{
+ const char FULLPATH[] = "mountpoint/not_a_dir/some_file.txt";
+ const char RELPATH[] = "not_a_dir";
+ EXPECT_LOOKUP(1, RELPATH)
+ .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) {
+ SET_OUT_HEADER_LEN(out, entry);
+ out->body.entry.entry_valid = UINT64_MAX;
+ out->body.entry.attr.mode = S_IFREG | 0644;
+ out->body.entry.nodeid = 42;
+ })));
+
+ ASSERT_EQ(-1, access(FULLPATH, F_OK));
+ ASSERT_EQ(ENOTDIR, errno);
+}
+
/*
* If lookup returns a non-zero entry timeout, then subsequent VOP_LOOKUPs
* should use the cached inode rather than requery the daemon
@@ -291,5 +306,3 @@ TEST_F(Lookup, subdir)
*/
ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno);
}
-
-
Modified: projects/fuse2/tests/sys/fs/fusefs/setattr.cc
==============================================================================
--- projects/fuse2/tests/sys/fs/fusefs/setattr.cc Wed Apr 10 20:44:54 2019 (r346100)
+++ projects/fuse2/tests/sys/fs/fusefs/setattr.cc Wed Apr 10 21:10:21 2019 (r346101)
@@ -41,7 +41,15 @@ using namespace testing;
class Setattr : public FuseTest {};
+class RofsSetattr: public Setattr {
+public:
+virtual void SetUp() {
+ m_ro = true;
+ Setattr::SetUp();
+}
+};
+
/*
* If setattr returns a non-zero cache timeout, then subsequent VOP_GETATTRs
* should use the cached attributes, rather than query the daemon
@@ -103,7 +111,6 @@ TEST_F(Setattr, chmod)
SET_OUT_HEADER_LEN(out, entry);
out->body.entry.attr.mode = S_IFREG | oldmode;
out->body.entry.nodeid = ino;
- out->body.entry.attr.mode = S_IFREG | oldmode;
})));
EXPECT_CALL(*m_mock, process(
@@ -554,4 +561,22 @@ TEST_F(Setattr, utimensat_mtime_only) {
<< strerror(errno);
}
-// TODO: test for erofs
+/* On a read-only mount, no attributes may be changed */
+TEST_F(RofsSetattr, erofs)
+{
+ const char FULLPATH[] = "mountpoint/some_file.txt";
+ const char RELPATH[] = "some_file.txt";
+ const uint64_t ino = 42;
+ const mode_t oldmode = 0755;
+ const mode_t newmode = 0644;
+
+ EXPECT_LOOKUP(1, RELPATH)
+ .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) {
+ SET_OUT_HEADER_LEN(out, entry);
+ out->body.entry.attr.mode = S_IFREG | oldmode;
+ out->body.entry.nodeid = ino;
+ })));
+
+ ASSERT_EQ(-1, chmod(FULLPATH, newmode));
+ ASSERT_EQ(EROFS, errno);
+}
Modified: projects/fuse2/tests/sys/fs/fusefs/xattr.cc
==============================================================================
--- projects/fuse2/tests/sys/fs/fusefs/xattr.cc Wed Apr 10 20:44:54 2019 (r346100)
+++ projects/fuse2/tests/sys/fs/fusefs/xattr.cc Wed Apr 10 21:10:21 2019 (r346101)
@@ -108,6 +108,13 @@ class Getxattr: public Xattr {};
class Listxattr: public Xattr {};
class Removexattr: public Xattr {};
class Setxattr: public Xattr {};
+class RofsXattr: public Xattr {
+public:
+virtual void SetUp() {
+ m_ro = true;
+ Xattr::SetUp();
+}
+};
/*
* If the extended attribute does not exist on this file, the daemon should
@@ -602,6 +609,32 @@ TEST_F(Setxattr, system)
r = extattr_set_file(FULLPATH, ns, "foo", (void*)value, value_len);
ASSERT_EQ(value_len, r) << strerror(errno);
+}
+
+TEST_F(RofsXattr, deleteextattr_erofs)
+{
+ uint64_t ino = 42;
+ int ns = EXTATTR_NAMESPACE_USER;
+
+ expect_lookup(RELPATH, ino, S_IFREG | 0644, 0, 1);
+
+ ASSERT_EQ(-1, extattr_delete_file(FULLPATH, ns, "foo"));
+ ASSERT_EQ(EROFS, errno);
+}
+
+TEST_F(RofsXattr, setextattr_erofs)
+{
+ uint64_t ino = 42;
+ const char value[] = "whatever";
+ ssize_t value_len = strlen(value) + 1;
+ int ns = EXTATTR_NAMESPACE_USER;
+ ssize_t r;
+
+ expect_lookup(RELPATH, ino, S_IFREG | 0644, 0, 1);
+
+ r = extattr_set_file(FULLPATH, ns, "foo", (void*)value, value_len);
+ ASSERT_EQ(-1, r);
+ EXPECT_EQ(EROFS, errno);
}
// TODO: EROFS tests
More information about the svn-src-projects
mailing list