Andy Lutomirski [off-list ref] writes:

On Mon, Dec 8, 2014 at 2:44 PM, Eric W. Biederman [off-list ref] wrote:

quoted

Andy Lutomirski [off-list ref] writes:

quoted

On Mon, Dec 8, 2014 at 2:11 PM, Eric W. Biederman [off-list ref] wrote:

quoted

- Expose the knob to user space through a proc file /proc/<pid>/setgroups

  A value of 0 means the setgroups system call is disabled in the

"deny"

quoted

  current processes user namespace and can not be enabled in the
  future in this user namespace.

  A value of 1 means the segtoups system call is enabled.

"allow"

quoted

- Descedent user namespaces inherit the value of setgroups from

s/Descedent/Descendent/

Bah.  I updated everything but the changelog comment.

quoted

quoted

--- a/kernel/groups.c
+++ b/kernel/groups.c

@@ -222,6 +222,7 @@ bool may_setgroups(void)
         * the user namespace has been established.
         */
        return userns_gid_mappings_established(user_ns) &&
+               userns_setgroups_allowed(user_ns) &&
                ns_capable(user_ns, CAP_SETGID);
 }

Can you add a comment explaining the ordering?  For example:

I need to think on what I can say to make it clear.
Perhaps: /* Careful the order of these checks is important. */

quoted

We need to check for a gid mapping before checking setgroups_allowed
because an unprivileged user can create a userns with setgroups
allowed, then disallow setgroups and add a mapping.  If we check in
the opposite order, then we have a race: we could see that setgroups
is allowed before the user clears the bit and then see that there is a
gid mapping after the other thread is done.

This text was actually my suggested comment text.

Now I see.

If you put smp_rmb() in this function with a comment like that, then I
think it will all make sense and be obviously correct (even with most
of the other barriers removed).

Right.

Given that we have to be careful when using these things anyway what
I was hoping to achieve with the barriers appears impossible, and
confusing so I will see about just adding barriers where we need them
for real.  Sigh.

Eric