From 6e0a3888541d08e34f02ccaf2a86940d266c9f01 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Thu, 31 Oct 2019 20:27:34 +0100
Subject: user-util: tweak to in_gid()

Let's make this robust towards parallel updates to group lists. This is
not going to happen IRL, but it makes me sleep better at night: let's
iterate a couple of times in case the list is updated while we are at
it.

Follow-up for: f5e0b942af1e86993c21f4e5c84342bb10403dac
---
 src/basic/user-util.c | 50 ++++++++++++++++++++++++++++++++++----------------
 1 file changed, 34 insertions(+), 16 deletions(-)

diff --git a/src/basic/user-util.c b/src/basic/user-util.c
index 7a31a69e36..b5fdfafd61 100644
--- a/src/basic/user-util.c
+++ b/src/basic/user-util.c
@@ -409,8 +409,10 @@ char* gid_to_name(gid_t gid) {
 }
 
 int in_gid(gid_t gid) {
-        gid_t *gids;
-        int ngroups, r, i;
+        _cleanup_free_ gid_t *allocated = NULL;
+        gid_t local[16], *p = local;
+        int ngroups = ELEMENTSOF(local);
+        unsigned attempt = 0;
 
         if (getgid() == gid)
                 return 1;
@@ -421,23 +423,39 @@ int in_gid(gid_t gid) {
         if (!gid_is_valid(gid))
                 return -EINVAL;
 
-        ngroups = getgroups(0, NULL);
-        if (ngroups < 0)
-                return -errno;
-        if (ngroups == 0)
-                return 0;
-
-        gids = newa(gid_t, ngroups);
+        for (;;) {
+                ngroups = getgroups(ngroups, p);
+                if (ngroups >= 0)
+                        break;
+                if (errno != EINVAL)
+                        return -errno;
+
+                /* Give up eventually */
+                if (attempt++ > 10)
+                        return -EINVAL;
+
+                /* Get actual size needed, and size the array explicitly. Note that this is potentially racy
+                 * to use (in multi-threaded programs), hence let's call this in a loop. */
+                ngroups = getgroups(0, NULL);
+                if (ngroups < 0)
+                        return -errno;
+                if (ngroups == 0)
+                        return false;
+
+                free(allocated);
+
+                allocated = new(gid_t, ngroups);
+                if (!allocated)
+                        return -ENOMEM;
 
-        r = getgroups(ngroups, gids);
-        if (r < 0)
-                return -errno;
+                p = allocated;
+        }
 
-        for (i = 0; i < r; i++)
-                if (gids[i] == gid)
-                        return 1;
+        for (int i = 0; i < ngroups; i++)
+                if (p[i] == gid)
+                        return true;
 
-        return 0;
+        return false;
 }
 
 int in_group(const char *name) {
-- 
cgit v1.2.1


From eb2cfa81b005264693cc15d156995f9c31034196 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Thu, 31 Oct 2019 20:28:49 +0100
Subject: test: add really basic in_gid() test

---
 src/test/test-user-util.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/test/test-user-util.c b/src/test/test-user-util.c
index 9475b99c28..47baacb518 100644
--- a/src/test/test-user-util.c
+++ b/src/test/test-user-util.c
@@ -286,6 +286,15 @@ static void test_make_salt(void) {
         assert(!streq(s, t));
 }
 
+static void test_in_gid(void) {
+
+        assert(in_gid(getgid()) >= 0);
+        assert(in_gid(getegid()) >= 0);
+
+        assert(in_gid(GID_INVALID) < 0);
+        assert(in_gid(TTY_GID) == 0); /* The TTY gid is for owning ttys, it would be really really weird if we were in it. */
+}
+
 int main(int argc, char *argv[]) {
         test_uid_to_name_one(0, "root");
         test_uid_to_name_one(UID_NOBODY, NOBODY_USER_NAME);
@@ -320,5 +329,7 @@ int main(int argc, char *argv[]) {
 
         test_make_salt();
 
+        test_in_gid();
+
         return 0;
 }
-- 
cgit v1.2.1