From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754976AbdBPRqn (ORCPT ); Thu, 16 Feb 2017 12:46:43 -0500 Received: from mx2.suse.de ([195.135.220.15]:52223 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754144AbdBPRqm (ORCPT ); Thu, 16 Feb 2017 12:46:42 -0500 From: Aleksa Sarai To: Andrew Morton , Alexey Dobriyan Cc: linux-kernel@vger.kernel.org, cyphar@cyphar.com, Aleksa Sarai , "Eric W. Biederman" , dev@opencontainers.org Subject: [PATCH] groups: don't return unmapped gids in getgroups(2) Date: Fri, 17 Feb 2017 04:47:50 +1100 Message-Id: <20170216174750.4995-1-asarai@suse.de> X-Mailer: git-send-email 2.11.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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. 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; -- 2.11.0