Re: getgrouplist(3) vs. getgroups(3)



Roland Illig wrote:
I updated the patch.

And I updated it once again. There's nothing completely new, just a few cosmetic changes. The changes are:

- geteuid() is used instead of getuid().
- getegid() is added to the groups[], even if getgroups() fails.
  But as it won't fail, there's no effective change.

For the other changes, see the detailed analysis below.

+    if (!initialized) {
+	uid = getuid ();
+	ngroups = getgroups (0, NULL);
+	if (ngroups != -1) {
+	    groups = g_new (gid_t, ngroups + 1);
+	    ngroups = getgroups (ngroups, groups);
+
+	    /* getgroups() may or may not return the effective group ID,
+	     * so we always include it at the end of the list. */
+	    if (ngroups >= 0) {
+		groups[ngroups++] = getegid();
+	    }
+	}
+	initialized = TRUE;
+    }

    if (!initialized) {
	uid = geteuid ();

-- Here I chose geteuid() instead of getuid(), because it's
-- the effective user ID which is used for the permissions.

	ngroups = getgroups (0, NULL);
	if (ngroups == -1)
	    ngroups = 0;	/* ignore errors */

-- It is very unlikely that getgroups() fails, and I could have
-- also used assert() for it. But for now, let's just assume
-- we have no additional groups if this call fails.

	/* allocate space for one element in addition to what
	 * will be filled by getgroups(). */
	groups = g_new (gid_t, ngroups + 1);

-- Nevertheless, also in the failing case we allocate a GID array,
-- which then will only contain one entry -- the EGID.

	if (ngroups != 0) {

-- Make sure getgroups() is not called with ngroups == 0, because
-- otherwise the return value might be > 0.

	    ngroups = getgroups (ngroups, groups);

-- Here I assume that the value returned by this call will not be
-- greater than the value from the call above. If there is no
-- thread calling setgroups() in between, this should definitely
-- be ok.

	    if (ngroups == -1)
		ngroups = 0;	/* ignore errors */
	}

-- groups[] has at least ngroups + 1 entries. ngroups of them are
-- filled by getgroups(), and the last one is still free.

	/* getgroups() may or may not return the effective group ID,
	 * so we always include it at the end of the list. */
	groups[ngroups++] = getegid ();

	initialized = TRUE;
    }


Roland



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]