From 9943d518721bb7056bd27ceaf6455ea98364219d Mon Sep 17 00:00:00 2001 From: blinke Date: Mon, 10 Aug 2015 15:31:44 +0200 Subject: [PATCH 1/4] configure.ac: added autoconf check for getgrouplist Signed-off-by: Yan, Zheng (cherry picked from commit 16b59c6cd68d532a47e8c0e7b1f088b41433f048) --- configure.ac | 3 +++ 1 file changed, 3 insertions(+) diff --git a/configure.ac b/configure.ac index 969baed8357db..3a82fbfeb7dda 100644 --- a/configure.ac +++ b/configure.ac @@ -932,6 +932,9 @@ AC_CHECK_FUNC([fallocate], [AC_DEFINE([CEPH_HAVE_FALLOCATE], [], [fallocate(2) is supported])], []) +# getgrouplist +AC_CHECK_FUNCS([getgrouplist]) + # # Test for time-related `struct stat` members. # From b0262b434dc82d8930ca3789bf06c56dcf5a315a Mon Sep 17 00:00:00 2001 From: blinke Date: Mon, 10 Aug 2015 17:43:10 +0200 Subject: [PATCH 2/4] client: added permission check based on getgrouplist Fixes: #13268 Signed-off-by: Yan, Zheng (cherry picked from commit f04c8da5432174874ca97d11a5b2fef56f95d73d) --- src/client/Client.cc | 47 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index 0d85db29ce8c0..9862f9b8d1423 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -90,6 +90,11 @@ using namespace std; #include "include/assert.h" #include "include/stat.h" +#if HAVE_GETGROUPLIST +#include +#include +#endif + #undef dout_prefix #define dout_prefix *_dout << "client." << whoami << " " @@ -4383,6 +4388,10 @@ void Client::handle_cap_grant(MetaSession *session, Inode *in, Cap *cap, MClient int Client::check_permissions(Inode *in, int flags, int uid, int gid) { + // initial number of group entries, defaults to posix standard of 16 + // PAM implementations may provide more than 16 groups.... + int initial_group_count = 16; + gid_t *sgids = NULL; int sgid_count = 0; if (getgroups_cb) { @@ -4392,11 +4401,45 @@ int Client::check_permissions(Inode *in, int flags, int uid, int gid) return sgid_count; } } +#if HAVE_GETGROUPLIST + else { + //use PAM to get the group list + sgid_count = initial_group_count; + sgids = (gid_t*)malloc(sgid_count * sizeof(gid_t)); + if (sgids == NULL) { + ldout(cct, 3) << "allocating group memory failed" << dendl; + return -EACCES; + } + struct passwd *pw; + pw = getpwuid(uid); + if (pw == NULL) { + ldout(cct, 3) << "getting user entry failed" << dendl; + return -EACCES; + } + while (1) { + if (getgrouplist(pw->pw_name, gid, sgids, &sgid_count) == -1) { + // we need to resize the group list and try again + sgids = (gid_t*)realloc(sgids, sgid_count * sizeof(gid_t)); + if (sgids == NULL) { + ldout(cct, 3) << "allocating group memory failed" << dendl; + return -EACCES; + } + continue; + } + // list was successfully retrieved + break; + } + } +#endif + // check permissions before doing anything else + int ret = 0; if (uid != 0 && !in->check_mode(uid, gid, sgids, sgid_count, flags)) { - return -EACCES; + ret = -EACCES; } - return 0; + if (sgids) + free(sgids); + return ret; } vinodeno_t Client::_get_vino(Inode *in) From a3e69a67fb6b5699cbd5fd06ce031bab9dceac57 Mon Sep 17 00:00:00 2001 From: Danny Al-Gaaf Date: Wed, 12 Aug 2015 18:38:38 +0200 Subject: [PATCH 3/4] client/Client.cc: fix realloc memory leak Fix handling of realloc. If realloc() fails it returns NULL, assigning the return value of realloc() directly to the pointer without checking for the result will lead to a memory leak. Signed-off-by: Danny Al-Gaaf Signed-off-by: Yan, Zheng (cherry picked from commit 4f98dab99c35663de89a06e2dfdbd874f56aed41) --- src/client/Client.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index 9862f9b8d1423..43131f8d257e1 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -4419,11 +4419,13 @@ int Client::check_permissions(Inode *in, int flags, int uid, int gid) while (1) { if (getgrouplist(pw->pw_name, gid, sgids, &sgid_count) == -1) { // we need to resize the group list and try again - sgids = (gid_t*)realloc(sgids, sgid_count * sizeof(gid_t)); - if (sgids == NULL) { + void *_realloc = NULL; + if ((_realloc = realloc(sgids, sgid_count * sizeof(gid_t))) == NULL) { ldout(cct, 3) << "allocating group memory failed" << dendl; + free(sgids); return -EACCES; } + sgids = (gid_t*)_realloc; continue; } // list was successfully retrieved From 19b76aafad9c87e1cd09ff1dd6b59bc8a30d8ac9 Mon Sep 17 00:00:00 2001 From: Danny Al-Gaaf Date: Tue, 18 Aug 2015 12:34:01 +0200 Subject: [PATCH 4/4] client/Client.cc: remove only once used variable Fix for: [src/client/Client.cc:4555]: (style) The scope of the variable 'initial_group_count' can be reduced. Signed-off-by: Danny Al-Gaaf Signed-off-by: Yan, Zheng (cherry picked from commit a29dd45dd89f59ff15018f541601ac5ede162174) --- src/client/Client.cc | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index 43131f8d257e1..a966b17021166 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -4388,10 +4388,6 @@ void Client::handle_cap_grant(MetaSession *session, Inode *in, Cap *cap, MClient int Client::check_permissions(Inode *in, int flags, int uid, int gid) { - // initial number of group entries, defaults to posix standard of 16 - // PAM implementations may provide more than 16 groups.... - int initial_group_count = 16; - gid_t *sgids = NULL; int sgid_count = 0; if (getgroups_cb) { @@ -4403,8 +4399,10 @@ int Client::check_permissions(Inode *in, int flags, int uid, int gid) } #if HAVE_GETGROUPLIST else { - //use PAM to get the group list - sgid_count = initial_group_count; + // use PAM to get the group list + // initial number of group entries, defaults to posix standard of 16 + // PAM implementations may provide more than 16 groups.... + sgid_count = 16; sgids = (gid_t*)malloc(sgid_count * sizeof(gid_t)); if (sgids == NULL) { ldout(cct, 3) << "allocating group memory failed" << dendl;