fusefs: allow the null chown and null chgrp

Even an unprivileged user should be able to chown a file to its current
owner, or chgrp it to its current group.  Those are no-ops.

Reported by:	pjdfstest
Sponsored by:	The FreeBSD Foundation
This commit is contained in:
Alan Somers 2019-05-07 01:27:23 +00:00
parent 1c8a5f5e39
commit 4e83d6555e
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/projects/fuse2/; revision=347217
2 changed files with 65 additions and 17 deletions

View File

@ -1519,15 +1519,21 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
struct thread *td = curthread;
struct mount *mp;
struct fuse_data *data;
struct vattr old_va;
int dataflags;
int err = 0;
int err = 0, err2;
accmode_t accmode = 0;
bool checkperm;
gid_t cr_gid;
mp = vnode_mount(vp);
data = fuse_get_mpdata(mp);
dataflags = data->dataflags;
checkperm = dataflags & FSESS_DEFAULT_PERMISSIONS;
if (cred->cr_ngroups > 0)
cr_gid = cred->cr_groups[0];
else
cr_gid = 0;
if (fuse_isdeadfs(vp)) {
return ENXIO;
@ -1537,10 +1543,20 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
if (checkperm) {
/* Only root may change a file's owner */
err = priv_check_cred(cred, PRIV_VFS_CHOWN);
if (err)
return err;;
}
accmode |= VADMIN;
if (err) {
/* As a special case, allow the null chown */
err2 = fuse_internal_getattr(vp, &old_va, cred,
td);
if (err2)
return (err2);
if (vap->va_uid != old_va.va_uid)
return err;
else
accmode |= VADMIN;
} else
accmode |= VADMIN;
} else
accmode |= VADMIN;
}
if (vap->va_gid != (gid_t)VNOVAL) {
if (checkperm && !groupmember(vap->va_gid, cred))
@ -1550,10 +1566,20 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
* groups
*/
err = priv_check_cred(cred, PRIV_VFS_CHOWN);
if (err)
return err;
}
accmode |= VADMIN;
if (err) {
/* As a special case, allow the null chgrp */
err2 = fuse_internal_getattr(vp, &old_va, cred,
td);
if (err2)
return (err2);
if (vap->va_gid != old_va.va_gid)
return err;
else
accmode |= VADMIN;
} else
accmode |= VADMIN;
} else
accmode |= VADMIN;
}
if (vap->va_size != VNOVAL) {
switch (vp->v_type) {
@ -1591,7 +1617,6 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
&& priv_check_cred(cred, PRIV_VFS_STICKYFILE))
return EFTYPE;
if (checkperm && (vap->va_mode & S_ISGID)) {
struct vattr old_va;
err = fuse_internal_getattr(vp, &old_va, cred, td);
if (err)
return (err);

View File

@ -294,6 +294,34 @@ TEST_F(Access, ok)
ASSERT_EQ(0, access(FULLPATH, access_mode)) << strerror(errno);
}
/* Unprivileged users may chown a file to their own uid */
TEST_F(Chown, chown_to_self)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";
const uint64_t ino = 42;
const mode_t mode = 0755;
uid_t uid;
uid = geteuid();
expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1, uid);
expect_lookup(RELPATH, ino, S_IFREG | mode, UINT64_MAX, uid);
/* The OS may optimize chown by omitting the redundant setattr */
EXPECT_CALL(*m_mock, process(
ResultOf([](auto in) {
return (in->header.opcode == FUSE_SETATTR);
}, Eq(true)),
_)
).WillRepeatedly(Invoke(ReturnImmediate([=](auto in __unused, auto out){
SET_OUT_HEADER_LEN(out, attr);
out->body.attr.attr.mode = S_IFREG | mode;
out->body.attr.attr.uid = uid;
})));
EXPECT_EQ(0, chown(FULLPATH, uid, -1)) << strerror(errno);
}
/* Only root may change a file's owner */
TEST_F(Chown, eperm)
{
@ -357,19 +385,14 @@ TEST_F(Chgrp, ok)
expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1, uid, gid);
expect_lookup(RELPATH, ino, S_IFREG | mode, UINT64_MAX, uid, gid);
EXPECT_CALL(*m_mock, process(
ResultOf([](auto in) {
return (in->header.opcode == FUSE_SETATTR);
}, Eq(true)),
_)
).Times(0);
/* The OS may optimize chgrp by omitting the redundant setattr */
EXPECT_CALL(*m_mock, process(
ResultOf([](auto in) {
return (in->header.opcode == FUSE_SETATTR &&
in->header.nodeid == ino);
}, Eq(true)),
_)
).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) {
).WillRepeatedly(Invoke(ReturnImmediate([=](auto in __unused, auto out){
SET_OUT_HEADER_LEN(out, attr);
out->body.attr.attr.mode = S_IFREG | mode;
out->body.attr.attr.uid = uid;