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 <xiubli@redhat.com>
(cherry picked from commit 2ab56ef9115d23514940f46dbdfa18cd6473d042)

diff --git a/src/client/Client.cc b/src/client/Client.cc
index e84ad8e4416b644c89cee7c2fcd1d164b820e17b..e1c1de294831eb451de10acc2ad7a937af109e75 100644 (file)
--- 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<C_SaferCond> 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 73a603397afdee4523137b5e698ccfc21cad9d88..f24bb8a37a6b47f131ae9433aef6495b96db751d 100644 (file)
--- 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 c266d339ae14283260e6ff86155e76bddc31d386..1a75a519333600ea78218f85850e90252b690045 100644 (file)
--- 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 e1c6e317b3cac54448dead102abde15412c7ea1c..62e0b51c2d31580e65039940b3f1ee089a922e84 100644 (file)
--- 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 fbac61e0870514881fca4b3d76ecf13eae290cb1..db3df7668ab702adf5de83c445351d2e486db4b5 100644 (file)
--- 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)