From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933265AbdBPSYW (ORCPT ); Thu, 16 Feb 2017 13:24:22 -0500 Received: from out01.mta.xmission.com ([166.70.13.231]:45716 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932948AbdBPSYU (ORCPT ); Thu, 16 Feb 2017 13:24:20 -0500 From: ebiederm@xmission.com (Eric W. Biederman) To: Aleksa Sarai Cc: Andrew Morton , Alexey Dobriyan , linux-kernel@vger.kernel.org, cyphar@cyphar.com, dev@opencontainers.org, , Linux Containers References: <20170216174750.4995-1-asarai@suse.de> Date: Fri, 17 Feb 2017 07:19:36 +1300 In-Reply-To: <20170216174750.4995-1-asarai@suse.de> (Aleksa Sarai's message of "Fri, 17 Feb 2017 04:47:50 +1100") Message-ID: <87a89mm213.fsf@xmission.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1ceQiy-0005AB-Rm;;;mid=<87a89mm213.fsf@xmission.com>;;;hst=in01.mta.xmission.com;;;ip=101.100.131.232;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX19msxn2iFbqgIKv1btmgv7NCbfnN/Bm3h4= X-SA-Exim-Connect-IP: 101.100.131.232 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.7 XMSubLong Long Subject * 1.5 TR_Symld_Words too many words that have symbols inside * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa01 1397; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_02 5+ unique symbols in subject * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: XMission; sa01 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: **;Aleksa Sarai X-Spam-Relay-Country: X-Spam-Timing: total 5783 ms - load_scoreonly_sql: 0.05 (0.0%), signal_user_changed: 7 (0.1%), b_tie_ro: 6 (0.1%), parse: 1.40 (0.0%), extract_message_metadata: 57 (1.0%), get_uri_detail_list: 4.4 (0.1%), tests_pri_-1000: 18 (0.3%), tests_pri_-950: 1.94 (0.0%), tests_pri_-900: 1.64 (0.0%), tests_pri_-400: 49 (0.8%), check_bayes: 47 (0.8%), b_tokenize: 24 (0.4%), b_tok_get_all: 12 (0.2%), b_comp_prob: 5 (0.1%), b_tok_touch_all: 2.9 (0.0%), b_finish: 0.78 (0.0%), tests_pri_0: 725 (12.5%), check_dkim_signature: 0.88 (0.0%), check_dkim_adsp: 4.3 (0.1%), tests_pri_500: 4915 (85.0%), poll_dns_idle: 4907 (84.9%), rewrite_mail: 0.00 (0.0%) Subject: Re: [PATCH] groups: don't return unmapped gids in getgroups(2) X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Added a few more relevant mailing-lists to the CC list. Aleksa Sarai writes: > One thing overlooked by commit 9cc46516ddf4 ("userns: Add a knob to > disable setgroups on a per user namespace basis") is that because > setgroups(2) no longer works in user namespaces it doesn't make any > sense to be returning weird group IDs that the process cannot do > anything with. This code works the same weather or not setgroups is enabled. > This change, along with the other changes made to require unprivileged > users to always disable setgroups(2), means that userspace programs such > as apt break inside rootless containers. While this change does change > the userspace ABI, any userspace program that has to deal with > getgroups(2) would have to filter out these "fake" group IDs anyway. > This just makes it so that less applications will have to handle this > broken API. Is it broken? Unless I am mistaken if we have a 16bit getgroups call and we 32bit group ids getgroups it behaves exactly the same way. The value we is (u16)-2. The traditional linux group id for this purpose. In all other contexts the best we can do for applications has been to return the user id or group id that says the value you are looking for does not fit in this context. Which makes me suspect this is not the right solution for getgroups. I don't know why apt breaks. You have not described that. Perhaps apt is seeing something misconfigured and complaining properly. I can be persauded but I need a better argument than this change makes one applicaiton work for me. Eric > Fixes: 9cc46516ddf4 ("userns: Add a knob to disable setgroups on a per user namespace basis") > Cc: "Eric W. Biederman" > Cc: > Signed-off-by: Aleksa Sarai > --- > kernel/groups.c | 25 ++++++++++++++++++------- > 1 file changed, 18 insertions(+), 7 deletions(-) > > diff --git a/kernel/groups.c b/kernel/groups.c > index 8dd7a61b7115..ebd01fff37d6 100644 > --- a/kernel/groups.c > +++ b/kernel/groups.c > @@ -41,16 +41,27 @@ static int groups_to_user(gid_t __user *grouplist, > const struct group_info *group_info) > { > struct user_namespace *user_ns = current_user_ns(); > - int i; > + int i, j = 0; > unsigned int count = group_info->ngroups; > > for (i = 0; i < count; i++) { > + kgid_t kgid = group_info->gid[i]; > gid_t gid; > - gid = from_kgid_munged(user_ns, group_info->gid[i]); > - if (put_user(gid, grouplist+i)) > + > + /* > + * Don't return unmapped gids, since there's nothing userspace > + * can do about them and they are very confusing -- since > + * setgroups(2) is disabled in user namespaces. > + */ > + if (!kgid_has_mapping(user_ns, kgid)) > + continue; > + > + gid = from_kgid(user_ns, kgid); > + if (put_user(gid, grouplist+j)) > return -EFAULT; > + j++; > } > - return 0; > + return j; > } > > /* fill a group_info from a user-space array - it must be allocated already */ > @@ -177,10 +188,10 @@ SYSCALL_DEFINE2(getgroups, int, gidsetsize, gid_t __user *, grouplist) > i = -EINVAL; > goto out; > } > - if (groups_to_user(grouplist, cred->group_info)) { > - i = -EFAULT; > + > + i = groups_to_user(grouplist, cred->group_info); > + if (i < 0) > goto out; > - } > } > out: > return i;