client/mds: have write codepath clear the setuid/setgid bits if they're set

Declare a new CEPH_SETATTR_KILL_SGUID flag. When that bit is set in
the mask, then that tells the server to clear out the S_ISUID and
S_ISGID bits. Doing that is less racy and problematic than trying
to do a read/modify/write cycle on the mode. Note that this flag is
ignored if the mode is being set in the same setattr call (uncommon,
but possible from something like ganesha or samba).

Then, change the client library write code to get As caps when issuing a
write, so we can check whether the mode has the setuid/setgid bits set.
If it does, then call __setattrx to clear them using
CEPH_SETATTR_KILL_SGUID.

Signed-off-by: Jeff Layton <jlayton@redhat.com>

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 129b47f0f1b473a0e5f312bd3e2e9cfc60ee8d1c..6f39c082ffea2fb30363dd0dafb99aa6d980143d 100644 (file)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -6494,7 +6494,9 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask,
   }
 
   if (in->caps_issued_mask(CEPH_CAP_AUTH_EXCL)) {
-    bool kill_sguid = false;
+    bool kill_sguid = mask & CEPH_SETATTR_KILL_SGUID;
+
+    mask &= ~CEPH_SETATTR_KILL_SGUID;
 
     if (mask & CEPH_SETATTR_UID) {
       in->ctime = ceph_clock_now(cct);
@@ -6571,6 +6573,9 @@ force_request:
   req->set_filepath(path);
   req->set_inode(in);
 
+  if (mask & CEPH_SETATTR_KILL_SGUID) {
+    req->inode_drop |= CEPH_CAP_AUTH_SHARED;
+  }
   if (mask & CEPH_SETATTR_MODE) {
     req->head.args.setattr.mode = stx->stx_mode;
     req->inode_drop |= CEPH_CAP_AUTH_SHARED;
@@ -8828,9 +8833,22 @@ int Client::_write(Fh *f, int64_t offset, uint64_t size, const char *buf,
   utime_t lat;
   uint64_t totalwritten;
   int have;
-  int r = get_caps(in, CEPH_CAP_FILE_WR, CEPH_CAP_FILE_BUFFER, &have, endoff);
-  if (r < 0) {
+  int r = get_caps(in, CEPH_CAP_FILE_WR|CEPH_CAP_AUTH_SHARED,
+                   CEPH_CAP_FILE_BUFFER, &have, endoff);
+  if (r < 0)
     return r;
+
+  /* clear the setuid/setgid bits, if any */
+  if (unlikely((in->mode & S_ISUID) ||
+              (in->mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP))) {
+    struct ceph_statx stx = { 0 };
+
+    put_cap_ref(in, CEPH_CAP_AUTH_SHARED);
+    r = __setattrx(in, &stx, CEPH_SETATTR_KILL_SGUID, f->actor_perms);
+    if (r < 0)
+      return r;
+  } else {
+    put_cap_ref(in, CEPH_CAP_AUTH_SHARED);
   }
 
   if (f->flags & O_DIRECT)

diff --git a/src/include/ceph_fs.h b/src/include/ceph_fs.h
index 946a10d4ba526f1f6bdfbbd1706d5dd55f418dff..ffc510813b90d2d88faabd2f791701128e770e71 100644 (file)
--- a/src/include/ceph_fs.h
+++ b/src/include/ceph_fs.h
@@ -380,6 +380,7 @@ extern const char *ceph_mds_op_name(int op);
 #endif
 #define CEPH_SETATTR_MTIME_NOW (1 << 7)
 #define CEPH_SETATTR_ATIME_NOW (1 << 8)
+#define CEPH_SETATTR_KILL_SGUID        (1 << 10)
 
 /*
  * Ceph setxattr request flags.

diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index 1293d7d0bc2adc310493c55c044dd14c7990263e..f1a9eed01bee7f83db1ef76aa9f52456c4c8d01e 100644 (file)
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -3740,7 +3740,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))
+  if (mask & (CEPH_SETATTR_MODE|CEPH_SETATTR_UID|CEPH_SETATTR_GID|CEPH_SETATTR_BTIME|CEPH_SETATTR_KILL_SGUID))
     xlocks.insert(&cur->authlock);
   if (mask & (CEPH_SETATTR_MTIME|CEPH_SETATTR_ATIME|CEPH_SETATTR_SIZE))
     xlocks.insert(&cur->filelock);
@@ -3800,7 +3800,7 @@ void Server::handle_client_setattr(MDRequestRef& mdr)
 
   if (mask & CEPH_SETATTR_MODE)
     pi->mode = (pi->mode & ~07777) | (req->head.args.setattr.mode & 07777);
-  else if ((mask & (CEPH_SETATTR_UID|CEPH_SETATTR_GID)) &&
+  else if ((mask & (CEPH_SETATTR_UID|CEPH_SETATTR_GID|CEPH_SETATTR_KILL_SGUID)) &&
            S_ISREG(pi->mode)) {
     pi->mode &= ~S_ISUID;
     if ((pi->mode & (S_ISGID|S_IXGRP)) == (S_ISGID|S_IXGRP))