From 6e0a3888541d08e34f02ccaf2a86940d266c9f01 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 31 Oct 2019 20:27:34 +0100 Subject: [PATCH 1/2] 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 | 48 +++++++++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/src/basic/user-util.c b/src/basic/user-util.c index 7a31a69e360..b5fdfafd612 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; + for (;;) { + ngroups = getgroups(ngroups, p); + if (ngroups >= 0) + break; + if (errno != EINVAL) + return -errno; - gids = newa(gid_t, ngroups); + /* Give up eventually */ + if (attempt++ > 10) + return -EINVAL; - r = getgroups(ngroups, gids); - if (r < 0) - return -errno; + /* 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; - for (i = 0; i < r; i++) - if (gids[i] == gid) - return 1; + free(allocated); - return 0; + allocated = new(gid_t, ngroups); + if (!allocated) + return -ENOMEM; + + p = allocated; + } + + for (int i = 0; i < ngroups; i++) + if (p[i] == gid) + return true; + + return false; } int in_group(const char *name) { From eb2cfa81b005264693cc15d156995f9c31034196 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 31 Oct 2019 20:28:49 +0100 Subject: [PATCH 2/2] 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 9475b99c280..47baacb518a 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; }