Re: [libvirt] [PATCHv2 2/3] util: add virGetGroupList

Since neither getpwuid_r() nor initgroups() are safe to call in between fork and exec (they obtain a mutex, but if some other thread in the parent also held the mutex at the time of the fork, the child will deadlock), we have to split out the functionality that is unsafe. At least glibc's initgroups() uses getgrouplist under the hood, so the ideal split is to expose getgrouplist for use before a fork. Gnulib already gives us a nice wrapper via mgetgroups; we wrap it once more to look up by uid instead of name. * .gnulib: Update for mgetgroups. * bootstrap.conf (gnulib_modules): Add mgetgroups. * src/util/virutil.h (virGetGroupList): New declaration. * src/util/virutil.c (virGetGroupList): New function. * src/libvirt_private.syms (virutil.h): Export it. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- .gnulib | 2 +- bootstrap.conf | 1 + src/libvirt_private.syms | 1 + src/util/virutil.c | 33 +++++++++++++++++++++++++++++++++ src/util/virutil.h | 2 ++ 5 files changed, 38 insertions(+), 1 deletion(-) diff --git a/.gnulib b/.gnulib index b72ff2a..da8d59e 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit b72ff2a45efde544c406804186d37a3254728571 +Subproject commit da8d59ee79138b85daee1ad5b22ea12e4fb6ce47

diff --git a/bootstrap.conf b/bootstrap.conf index 410e4f5..1c59c22 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -71,6 +71,7 @@ listen localeconv maintainer-makefile manywarnings +mgetgroups mkdtemp mkostemp mkostemps diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6df5500..d8b03f1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2002,6 +2002,7 @@ virGetDeviceID; virGetDeviceUnprivSGIO; virGetFCHostNameByWWN; virGetGroupID; +virGetGroupList; virGetGroupName; virGetHostname; virGetUnprivSGIOSysfsPath; diff --git a/src/util/virutil.c b/src/util/virutil.c index de7b532..844c694 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -67,6 +67,7 @@ #endif #include "c-ctype.h" +#include "mgetgroups.h" #include "virerror.h" #include "virlog.h" #include "virbuffer.h" @@ -990,6 +991,38 @@ virGetGroupID(const char *group, gid_t *gid) return 0; } + +/* Compute the list of supplementary groups associated with @uid, and + * including @gid in the list (unless it is -1), storing a malloc'd + * result into @list. Return the size of the list on success, or -1 + * on failure with error reported and errno set. May not be called + * between fork and exec. */ +int +virGetGroupList(uid_t uid, gid_t gid, gid_t **list) +{ + int ret = -1; + char *user = NULL; + gid_t group; + + *list = NULL; + if (uid == (uid_t)-1) + return 0; + + if (!(user = virGetUserEnt(uid, VIR_USER_ENT_NAME, &group))) + return -1; + + if (gid == (gid_t)-1) + gid = group; + + ret = mgetgroups(user, gid, list); + if (ret < 0) + virReportSystemError(errno, + _("cannot get group list for '%s'"), user); + VIR_FREE(user); + return ret; +} + + /* Set the real and effective uid and gid to the given values, and call * initgroups so that the process has all the assumed group membership of * that uid. return 0 on success, -1 on failure (the original system error diff --git a/src/util/virutil.h b/src/util/virutil.h index 280a18d..6480b07 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -115,6 +115,8 @@ char *virGetUserCacheDirectory(void); char *virGetUserRuntimeDirectory(void); char *virGetUserName(uid_t uid); char *virGetGroupName(gid_t gid); +int virGetGroupList(uid_t uid, gid_t group, gid_t **groups) + ATTRIBUTE_NONNULL(3); int virGetUserID(const char *name, uid_t *uid) ATTRIBUTE_RETURN_CHECK; int virGetGroupID(const char *name,