From: Jeff Layton <jlayton@redhat.com>
Date: Thu, 8 Dec 2016 19:24:04 +0000 (-0500)
Subject: client/mds: have write codepath clear the setuid/setgid bits if they're set
X-Git-Tag: v11.1.1~22^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=2d1c91f490331492cd9195bc63ca49eae545bbbf;p=ceph.git

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 129b47f0f1b..6f39c082ffe 100644
--- 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 946a10d4ba5..ffc510813b9 100644
--- 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 1293d7d0bc2..f1a9eed01be 100644
--- 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))