From 4e843ad78b26e6b33581ca2689b1cd1cd368fc3a Mon Sep 17 00:00:00 2001 From: Xiubo Li Date: Thu, 9 Feb 2023 19:49:28 +0800 Subject: [PATCH] mds/client: clear the suid/sgid in fallocate path There is no Posix item requires that we should clear the suid/sgid in fallocate code path but this is the default behaviour for most of the filesystems and the VFS layer. And also the same for the write code path, which have already support it. Fixes: https://tracker.ceph.com/issues/58680 Signed-off-by: Xiubo Li (cherry picked from commit 2ab56ef9115d23514940f46dbdfa18cd6473d042) Conflicts: - conflicts with dependent commit a35439373eba907b131075929e41e562c1eeab97 (mds: allow setattr to change fscrypt_auth/file) --- src/client/Client.cc | 62 +++++++++++++++++++++++++++++----- src/client/Client.h | 1 + src/include/ceph_fs.h | 26 ++++++++------ src/include/cephfs/libcephfs.h | 25 ++++++++------ src/mds/Server.cc | 20 ++++++++--- 5 files changed, 100 insertions(+), 34 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index f8063b46e5d5..5b0ccfa34fd8 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -7777,8 +7777,6 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, if (in->caps_issued_mask(CEPH_CAP_AUTH_EXCL)) { kill_sguid = mask & (CEPH_SETATTR_SIZE|CEPH_SETATTR_KILL_SGUID); - - mask &= ~CEPH_SETATTR_KILL_SGUID; } else if (mask & CEPH_SETATTR_SIZE) { /* If we don't have Ax, then we must ask the server to clear them on truncate */ mask |= CEPH_SETATTR_KILL_SGUID; @@ -7842,11 +7840,18 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, } else { mask &= ~CEPH_SETATTR_MODE; } - } else if (in->caps_issued_mask(CEPH_CAP_AUTH_EXCL) && - kill_sguid && S_ISREG(in->mode) && - (in->mode & (S_IXUSR|S_IXGRP|S_IXOTH))) { - /* Must squash the any setuid/setgid bits with an ownership change */ - in->mode &= ~(S_ISUID|S_ISGID); + } else if (in->caps_issued_mask(CEPH_CAP_AUTH_EXCL) && S_ISREG(in->mode)) { + if (kill_sguid && (in->mode & (S_IXUSR|S_IXGRP|S_IXOTH))) { + in->mode &= ~(S_ISUID|S_ISGID); + } else { + if (mask & CEPH_SETATTR_KILL_SUID) { + in->mode &= ~S_ISUID; + } + if (mask & CEPH_SETATTR_KILL_SGID) { + in->mode &= ~S_ISGID; + } + } + mask &= ~(CEPH_SETATTR_KILL_SGUID|CEPH_SETATTR_KILL_SUID|CEPH_SETATTR_KILL_SGID); in->mark_caps_dirty(CEPH_CAP_AUTH_EXCL); } @@ -14790,6 +14795,41 @@ int Client::ll_sync_inode(Inode *in, bool syncdataonly) return _fsync(in, syncdataonly); } +int Client::clear_suid_sgid(Inode *in, const UserPerm& perms) +{ + ldout(cct, 20) << __func__ << " " << *in << "; " << perms << dendl; + + if (!in->is_file()) { + return 0; + } + + if (likely(!(in->mode & (S_ISUID|S_ISGID)))) { + return 0; + } + + if (perms.uid() == 0 || perms.uid() == in->uid) { + return 0; + } + + int mask; + + // always drop the suid + if (unlikely(in->mode & S_ISUID)) { + mask = CEPH_SETATTR_KILL_SUID; + } + + // remove the sgid if S_IXUGO is set or the inode is + // is not in the caller's group list. + if ((in->mode & S_ISGID) && + ((in->mode & S_IXUGO) || !perms.gid_in_groups(in->gid))) { + mask |= CEPH_SETATTR_KILL_SGID; + } + + ldout(cct, 20) << __func__ << " mask " << mask << dendl; + struct ceph_statx stx = { 0 }; + return __setattrx(in, &stx, mask, perms); +} + int Client::_fallocate(Fh *fh, int mode, int64_t offset, int64_t length) { ceph_assert(ceph_mutex_is_locked_by_me(client_lock)); @@ -14828,6 +14868,12 @@ int Client::_fallocate(Fh *fh, int mode, int64_t offset, int64_t length) if (r < 0) return r; + r = clear_suid_sgid(in, fh->actor_perms); + if (r < 0) { + put_cap_ref(in, CEPH_CAP_FILE_WR); + return r; + } + std::unique_ptr onuninline = nullptr; if (mode & FALLOC_FL_PUNCH_HOLE) { if (in->inline_version < CEPH_INLINE_NONE && @@ -14932,7 +14978,7 @@ int Client::fallocate(int fd, int mode, loff_t offset, loff_t length) if (!mref_reader.is_state_satisfied()) return -CEPHFS_ENOTCONN; - tout(cct) << __func__ << " " << " " << fd << mode << " " << offset << " " << length << std::endl; + tout(cct) << __func__ << " " << fd << mode << " " << offset << " " << length << std::endl; std::scoped_lock lock(client_lock); Fh *fh = get_filehandle(fd); diff --git a/src/client/Client.h b/src/client/Client.h index 497f016da3ec..ad5469aa30e6 100644 --- a/src/client/Client.h +++ b/src/client/Client.h @@ -1385,6 +1385,7 @@ private: int _fsync(Fh *fh, bool syncdataonly); int _fsync(Inode *in, bool syncdataonly); int _sync_fs(); + int clear_suid_sgid(Inode *in, const UserPerm& perms); int _fallocate(Fh *fh, int mode, int64_t offset, int64_t length); int _getlk(Fh *fh, struct flock *fl, uint64_t owner); int _setlk(Fh *fh, struct flock *fl, uint64_t owner, int sleep); diff --git a/src/include/ceph_fs.h b/src/include/ceph_fs.h index 491931a8b278..660ca88c4e43 100644 --- a/src/include/ceph_fs.h +++ b/src/include/ceph_fs.h @@ -429,18 +429,22 @@ enum { extern const char *ceph_mds_op_name(int op); #ifndef CEPH_SETATTR_MODE -#define CEPH_SETATTR_MODE (1 << 0) -#define CEPH_SETATTR_UID (1 << 1) -#define CEPH_SETATTR_GID (1 << 2) -#define CEPH_SETATTR_MTIME (1 << 3) -#define CEPH_SETATTR_ATIME (1 << 4) -#define CEPH_SETATTR_SIZE (1 << 5) -#define CEPH_SETATTR_CTIME (1 << 6) -#define CEPH_SETATTR_MTIME_NOW (1 << 7) -#define CEPH_SETATTR_ATIME_NOW (1 << 8) -#define CEPH_SETATTR_BTIME (1 << 9) +#define CEPH_SETATTR_MODE (1 << 0) +#define CEPH_SETATTR_UID (1 << 1) +#define CEPH_SETATTR_GID (1 << 2) +#define CEPH_SETATTR_MTIME (1 << 3) +#define CEPH_SETATTR_ATIME (1 << 4) +#define CEPH_SETATTR_SIZE (1 << 5) +#define CEPH_SETATTR_CTIME (1 << 6) +#define CEPH_SETATTR_MTIME_NOW (1 << 7) +#define CEPH_SETATTR_ATIME_NOW (1 << 8) +#define CEPH_SETATTR_BTIME (1 << 9) +#define CEPH_SETATTR_KILL_SGUID (1 << 10) +#define CEPH_SETATTR_FSCRYPT_AUTH (1 << 11) +#define CEPH_SETATTR_FSCRYPT_FILE (1 << 12) +#define CEPH_SETATTR_KILL_SUID (1 << 13) +#define CEPH_SETATTR_KILL_SGID (1 << 14) #endif -#define CEPH_SETATTR_KILL_SGUID (1 << 10) /* * open request flags diff --git a/src/include/cephfs/libcephfs.h b/src/include/cephfs/libcephfs.h index 185d5e40e04b..45a110338b33 100644 --- a/src/include/cephfs/libcephfs.h +++ b/src/include/cephfs/libcephfs.h @@ -113,16 +113,21 @@ struct snap_info { /* setattr mask bits */ #ifndef CEPH_SETATTR_MODE -# define CEPH_SETATTR_MODE 1 -# define CEPH_SETATTR_UID 2 -# define CEPH_SETATTR_GID 4 -# define CEPH_SETATTR_MTIME 8 -# define CEPH_SETATTR_ATIME 16 -# define CEPH_SETATTR_SIZE 32 -# define CEPH_SETATTR_CTIME 64 -# define CEPH_SETATTR_MTIME_NOW 128 -# define CEPH_SETATTR_ATIME_NOW 256 -# define CEPH_SETATTR_BTIME 512 +#define CEPH_SETATTR_MODE (1 << 0) +#define CEPH_SETATTR_UID (1 << 1) +#define CEPH_SETATTR_GID (1 << 2) +#define CEPH_SETATTR_MTIME (1 << 3) +#define CEPH_SETATTR_ATIME (1 << 4) +#define CEPH_SETATTR_SIZE (1 << 5) +#define CEPH_SETATTR_CTIME (1 << 6) +#define CEPH_SETATTR_MTIME_NOW (1 << 7) +#define CEPH_SETATTR_ATIME_NOW (1 << 8) +#define CEPH_SETATTR_BTIME (1 << 9) +#define CEPH_SETATTR_KILL_SGUID (1 << 10) +#define CEPH_SETATTR_FSCRYPT_AUTH (1 << 11) +#define CEPH_SETATTR_FSCRYPT_FILE (1 << 12) +#define CEPH_SETATTR_KILL_SUID (1 << 13) +#define CEPH_SETATTR_KILL_SGID (1 << 14) #endif /* define error codes for the mount function*/ diff --git a/src/mds/Server.cc b/src/mds/Server.cc index 270e17ae063b..697461ab391d 100644 --- a/src/mds/Server.cc +++ b/src/mds/Server.cc @@ -5109,7 +5109,7 @@ void Server::handle_client_setattr(MDRequestRef& mdr) __u32 access_mask = MAY_WRITE; // xlock inode - if (mask & (CEPH_SETATTR_MODE|CEPH_SETATTR_UID|CEPH_SETATTR_GID|CEPH_SETATTR_BTIME|CEPH_SETATTR_KILL_SGUID)) + if (mask & (CEPH_SETATTR_MODE|CEPH_SETATTR_UID|CEPH_SETATTR_GID|CEPH_SETATTR_BTIME|CEPH_SETATTR_KILL_SGUID|CEPH_SETATTR_KILL_SUID|CEPH_SETATTR_KILL_SGID)) lov.add_xlock(&cur->authlock); if (mask & (CEPH_SETATTR_MTIME|CEPH_SETATTR_ATIME|CEPH_SETATTR_SIZE)) lov.add_xlock(&cur->filelock); @@ -5169,10 +5169,20 @@ void Server::handle_client_setattr(MDRequestRef& mdr) if (mask & CEPH_SETATTR_MODE) pi.inode->mode = (pi.inode->mode & ~07777) | (req->head.args.setattr.mode & 07777); - else if ((mask & (CEPH_SETATTR_UID|CEPH_SETATTR_GID|CEPH_SETATTR_KILL_SGUID)) && - S_ISREG(pi.inode->mode) && - (pi.inode->mode & (S_IXUSR|S_IXGRP|S_IXOTH))) { - pi.inode->mode &= ~(S_ISUID|S_ISGID); + else if ((mask & (CEPH_SETATTR_UID|CEPH_SETATTR_GID|CEPH_SETATTR_KILL_SGUID| + CEPH_SETATTR_KILL_SUID|CEPH_SETATTR_KILL_SGID)) && + S_ISREG(pi.inode->mode)) { + if (mask & (CEPH_SETATTR_UID|CEPH_SETATTR_GID|CEPH_SETATTR_KILL_SGUID) && + (pi.inode->mode & (S_IXUSR|S_IXGRP|S_IXOTH))) { + pi.inode->mode &= ~(S_ISUID|S_ISGID); + } else { + if (mask & CEPH_SETATTR_KILL_SUID) { + pi.inode->mode &= ~S_ISUID; + } + if (mask & CEPH_SETATTR_KILL_SGID) { + pi.inode->mode &= ~S_ISGID; + } + } } if (mask & CEPH_SETATTR_MTIME) -- 2.47.3