fuse: handle errors appropriately when getting group list

Currently, the GET_GROUPS macro just ignores errors that occur when
fetching the current task's groups. The callers then end up casting
the error to a count.

Change the macro to a real function, and just have it cleanly ignore
errors when getting groups fails. We do log the error to the debug
log however in that case.

Tracker: http://tracker.ceph.com/issues/23446
Signed-off-by: Jeff Layton <jlayton@redhat.com>

diff --git a/src/client/fuse_ll.cc b/src/client/fuse_ll.cc
index 49597b7bb32eaee52d9c726bd40bad4ab01c6d24..49f769cf029db5628f1e7a77984b7ad30854dbaf 100644 (file)
--- a/src/client/fuse_ll.cc
+++ b/src/client/fuse_ll.cc
@@ -136,13 +136,21 @@ static int getgroups_cb(void *handle, gid_t **sgids)
   return getgroups(req, sgids);
 }
 
-#define GET_GROUPS(perms, req) {                               \
-  if (g_conf->get_val<bool>("fuse_set_user_groups")) { \
-    gid_t *gids = NULL;                                                \
-    int count = getgroups(req, &gids);                         \
-    perms.init_gids(gids, count);                              \
-    perms.take_gids();                                         \
-  } }
+static void get_fuse_groups(UserPerm& perms, fuse_req_t req)
+{
+  if (g_conf->get_val<bool>("fuse_set_user_groups")) {
+    gid_t *gids = NULL;
+    int count = getgroups(req, &gids);
+
+    if (count > 0) {
+      perms.init_gids(gids, count);
+      perms.take_gids();
+    } else if (count < 0) {
+      derr << __func__ << ": getgroups failed: " << cpp_strerror(-count)
+          << dendl;
+    }
+  }
+}
 
 
 static CephFuse::Handle *fuse_ll_req_prepare(fuse_req_t req)
@@ -160,7 +168,7 @@ static void fuse_ll_lookup(fuse_req_t req, fuse_ino_t parent, const char *name)
   Inode *i2, *i1 = cfuse->iget(parent); // see below
   int r;
   UserPerm perms(ctx->uid, ctx->gid);
-  GET_GROUPS(perms, req);
+  get_fuse_groups(perms, req);
 
   if (!i1)
   {
@@ -202,7 +210,7 @@ static void fuse_ll_getattr(fuse_req_t req, fuse_ino_t ino,
   Inode *in = cfuse->iget(ino);
   struct stat stbuf;
   UserPerm perms(ctx->uid, ctx->gid);
-  GET_GROUPS(perms, req);
+  get_fuse_groups(perms, req);
   
   (void) fi; // XXX
 
@@ -224,7 +232,7 @@ static void fuse_ll_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr,
   const struct fuse_ctx *ctx = fuse_req_ctx(req);
   Inode *in = cfuse->iget(ino);
   UserPerm perms(ctx->uid, ctx->gid);
-  GET_GROUPS(perms, req);
+  get_fuse_groups(perms, req);
 
   int mask = 0;
   if (to_set & FUSE_SET_ATTR_MODE) mask |= CEPH_SETATTR_MODE;
@@ -261,7 +269,7 @@ static void fuse_ll_setxattr(fuse_req_t req, fuse_ino_t ino, const char *name,
   const struct fuse_ctx *ctx = fuse_req_ctx(req);
   Inode *in = cfuse->iget(ino);
   UserPerm perms(ctx->uid, ctx->gid);
-  GET_GROUPS(perms, req);
+  get_fuse_groups(perms, req);
 
   int r = cfuse->client->ll_setxattr(in, name, value, size, flags, perms);
   fuse_reply_err(req, -r);
@@ -276,7 +284,7 @@ static void fuse_ll_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size)
   Inode *in = cfuse->iget(ino);
   char buf[size];
   UserPerm perms(ctx->uid, ctx->gid);
-  GET_GROUPS(perms, req);
+  get_fuse_groups(perms, req);
 
   int r = cfuse->client->ll_listxattr(in, buf, size, perms);
   if (size == 0 && r >= 0)
@@ -301,7 +309,7 @@ static void fuse_ll_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name,
   Inode *in = cfuse->iget(ino);
   char buf[size];
   UserPerm perms(ctx->uid, ctx->gid);
-  GET_GROUPS(perms, req);
+  get_fuse_groups(perms, req);
 
   int r = cfuse->client->ll_getxattr(in, name, buf, size, perms);
   if (size == 0 && r >= 0)
@@ -321,7 +329,7 @@ static void fuse_ll_removexattr(fuse_req_t req, fuse_ino_t ino,
   const struct fuse_ctx *ctx = fuse_req_ctx(req);
   Inode *in = cfuse->iget(ino);
   UserPerm perms(ctx->uid, ctx->gid);
-  GET_GROUPS(perms, req);
+  get_fuse_groups(perms, req);
 
   int r = cfuse->client->ll_removexattr(in, name, perms);
   fuse_reply_err(req, -r);
@@ -338,7 +346,7 @@ static void fuse_ll_opendir(fuse_req_t req, fuse_ino_t ino,
   void *dirp;
 
   UserPerm perms(ctx->uid, ctx->gid);
-  GET_GROUPS(perms, req);
+  get_fuse_groups(perms, req);
 
   int r = cfuse->client->ll_opendir(in, fi->flags, (dir_result_t **)&dirp,
                                    perms);
@@ -359,7 +367,7 @@ static void fuse_ll_readlink(fuse_req_t req, fuse_ino_t ino)
   Inode *in = cfuse->iget(ino);
   char buf[PATH_MAX + 1];  // leave room for a null terminator
   UserPerm perms(ctx->uid, ctx->gid);
-  GET_GROUPS(perms, req);
+  get_fuse_groups(perms, req);
 
   int r = cfuse->client->ll_readlink(in, buf, sizeof(buf) - 1, perms);
   if (r >= 0) {
@@ -380,7 +388,7 @@ static void fuse_ll_mknod(fuse_req_t req, fuse_ino_t parent, const char *name,
   Inode *i2, *i1 = cfuse->iget(parent);
   struct fuse_entry_param fe;
   UserPerm perms(ctx->uid, ctx->gid);
-  GET_GROUPS(perms, req);
+  get_fuse_groups(perms, req);
 
   memset(&fe, 0, sizeof(fe));
 
@@ -409,7 +417,7 @@ static void fuse_ll_mkdir(fuse_req_t req, fuse_ino_t parent, const char *name,
 
   memset(&fe, 0, sizeof(fe));
   UserPerm perm(ctx->uid, ctx->gid);
-  GET_GROUPS(perm, req);
+  get_fuse_groups(perm, req);
 #ifdef HAVE_SYS_SYNCFS
   auto fuse_multithreaded = cfuse->client->cct->_conf->get_val<bool>(
     "fuse_multithreaded");
@@ -455,7 +463,7 @@ static void fuse_ll_unlink(fuse_req_t req, fuse_ino_t parent, const char *name)
   const struct fuse_ctx *ctx = fuse_req_ctx(req);
   Inode *in = cfuse->iget(parent);
   UserPerm perm(ctx->uid, ctx->gid);
-  GET_GROUPS(perm, req);
+  get_fuse_groups(perm, req);
 
   int r = cfuse->client->ll_unlink(in, name, perm);
   fuse_reply_err(req, -r);
@@ -469,7 +477,7 @@ static void fuse_ll_rmdir(fuse_req_t req, fuse_ino_t parent, const char *name)
   const struct fuse_ctx *ctx = fuse_req_ctx(req);
   Inode *in = cfuse->iget(parent);
   UserPerm perms(ctx->uid, ctx->gid);
-  GET_GROUPS(perms, req);
+  get_fuse_groups(perms, req);
 
   int r = cfuse->client->ll_rmdir(in, name, perms);
   fuse_reply_err(req, -r);
@@ -485,7 +493,7 @@ static void fuse_ll_symlink(fuse_req_t req, const char *existing,
   Inode *i2, *i1 = cfuse->iget(parent);
   struct fuse_entry_param fe;
   UserPerm perms(ctx->uid, ctx->gid);
-  GET_GROUPS(perms, req);
+  get_fuse_groups(perms, req);
 
   memset(&fe, 0, sizeof(fe));
 
@@ -511,7 +519,7 @@ static void fuse_ll_rename(fuse_req_t req, fuse_ino_t parent, const char *name,
   Inode *in = cfuse->iget(parent);
   Inode *nin = cfuse->iget(newparent);
   UserPerm perm(ctx->uid, ctx->gid);
-  GET_GROUPS(perm, req);
+  get_fuse_groups(perm, req);
 
   int r = cfuse->client->ll_rename(in, name, nin, newname, perm);
   fuse_reply_err(req, -r);
@@ -531,7 +539,7 @@ static void fuse_ll_link(fuse_req_t req, fuse_ino_t ino, fuse_ino_t newparent,
 
   memset(&fe, 0, sizeof(fe));
   UserPerm perm(ctx->uid, ctx->gid);
-  GET_GROUPS(perm, req);
+  get_fuse_groups(perm, req);
   
   /*
    * Note that we could successfully link, but then fail the subsequent
@@ -570,7 +578,7 @@ static void fuse_ll_open(fuse_req_t req, fuse_ino_t ino,
   Inode *in = cfuse->iget(ino);
   Fh *fh = NULL;
   UserPerm perms(ctx->uid, ctx->gid);
-  GET_GROUPS(perms, req);
+  get_fuse_groups(perms, req);
 
   int r = cfuse->client->ll_open(in, fi->flags, &fh, perms);
   if (r == 0) {
@@ -778,7 +786,7 @@ static void fuse_ll_create(fuse_req_t req, fuse_ino_t parent, const char *name,
   struct fuse_entry_param fe;
   Fh *fh = NULL;
   UserPerm perms(ctx->uid, ctx->gid);
-  GET_GROUPS(perms, req);
+  get_fuse_groups(perms, req);
 
   memset(&fe, 0, sizeof(fe));
 
@@ -813,7 +821,7 @@ static void fuse_ll_statfs(fuse_req_t req, fuse_ino_t ino)
   Inode *in = cfuse->iget(ino);
   const struct fuse_ctx *ctx = fuse_req_ctx(req);
   UserPerm perms(ctx->uid, ctx->gid);
-  GET_GROUPS(perms, req);
+  get_fuse_groups(perms, req);
 
   int r = cfuse->client->ll_statfs(in, &stbuf, perms);
   if (r == 0)