From: Xiubo Li Date: Thu, 9 Feb 2023 11:49:28 +0000 (+0800) Subject: mds/client: clear the suid/sgid in fallocate path X-Git-Tag: v18.1.2~8^2~8 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=fc56566afbdd7741fa3d9fd86f563f151671a5c4;p=ceph.git 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) --- diff --git a/src/client/Client.cc b/src/client/Client.cc index e84ad8e4416b..e1c1de294831 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -7797,8 +7797,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; @@ -7862,11 +7860,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); } @@ -14920,6 +14925,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)); @@ -14958,6 +14998,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 && @@ -15062,7 +15108,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 73a603397afd..f24bb8a37a6b 100644 --- a/src/client/Client.h +++ b/src/client/Client.h @@ -1389,6 +1389,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 c266d339ae14..1a75a5193336 100644 --- a/src/include/ceph_fs.h +++ b/src/include/ceph_fs.h @@ -446,6 +446,8 @@ extern const char *ceph_mds_op_name(int op); #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 /* diff --git a/src/include/cephfs/libcephfs.h b/src/include/cephfs/libcephfs.h index e1c6e317b3ca..62e0b51c2d31 100644 --- a/src/include/cephfs/libcephfs.h +++ b/src/include/cephfs/libcephfs.h @@ -127,6 +127,8 @@ struct snap_info { #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 fbac61e08705..db3df7668ab7 100644 --- a/src/mds/Server.cc +++ b/src/mds/Server.cc @@ -5158,7 +5158,7 @@ void Server::handle_client_setattr(MDRequestRef& mdr) } // xlock inode - if (mask & (CEPH_SETATTR_MODE|CEPH_SETATTR_UID|CEPH_SETATTR_GID|CEPH_SETATTR_BTIME|CEPH_SETATTR_KILL_SGUID|CEPH_SETATTR_FSCRYPT_AUTH)) + if (mask & (CEPH_SETATTR_MODE|CEPH_SETATTR_UID|CEPH_SETATTR_GID|CEPH_SETATTR_BTIME|CEPH_SETATTR_KILL_SGUID|CEPH_SETATTR_FSCRYPT_AUTH|CEPH_SETATTR_KILL_SUID|CEPH_SETATTR_KILL_SGID)) lov.add_xlock(&cur->authlock); if (mask & (CEPH_SETATTR_MTIME|CEPH_SETATTR_ATIME|CEPH_SETATTR_SIZE|CEPH_SETATTR_FSCRYPT_FILE)) lov.add_xlock(&cur->filelock); @@ -5252,10 +5252,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)