Andy Lutomirski <luto@xxxxxxxxxxxxxx> writes: > On Mon, Dec 8, 2014 at 2:44 PM, Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote: >> Andy Lutomirski <luto@xxxxxxxxxxxxxx> writes: >> >>> On Mon, Dec 8, 2014 at 2:11 PM, Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote: >>>> >>>> - 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" >>> >>>> 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" >>> >>>> - Descedent user namespaces inherit the value of setgroups from >>> >>> s/Descedent/Descendent/ >> >> Bah. I updated everything but the changelog comment. >> >>>> --- 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. */ >> >>> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
