From 2d1c91f490331492cd9195bc63ca49eae545bbbf Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Thu, 8 Dec 2016 14:24:04 -0500 Subject: [PATCH] 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 --- src/client/Client.cc | 24 +++++++++++++++++++++--- src/include/ceph_fs.h | 1 + src/mds/Server.cc | 4 ++-- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index 129b47f0f1b4..6f39c082ffea 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 946a10d4ba52..ffc510813b90 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 1293d7d0bc2a..f1a9eed01bee 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)) -- 2.47.3