mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Aleksa Sarai <asarai@suse.de>
To: Andrew Morton <akpm@linux-foundation.org>,
	Alexey Dobriyan <adobriyan@gmail.com>
Cc: linux-kernel@vger.kernel.org, cyphar@cyphar.com,
	Aleksa Sarai <asarai@suse.de>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	dev@opencontainers.org
Subject: [PATCH] groups: don't return unmapped gids in getgroups(2)
Date: Fri, 17 Feb 2017 04:47:50 +1100	[thread overview]
Message-ID: <20170216174750.4995-1-asarai@suse.de> (raw)

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" <ebiederm@xmission.com>
Cc: <dev@opencontainers.org>
Signed-off-by: Aleksa Sarai <asarai@suse.de>
---
 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

             reply	other threads:[~2017-02-16 17:46 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-16 17:47 Aleksa Sarai [this message]
2017-02-16 18:19 ` Eric W. Biederman
2017-02-17  8:44   ` Aleksa Sarai
2017-02-17 17:09     ` Andy Lutomirski
2017-02-17 17:53       ` Aleksa Sarai
2017-02-17 19:42         ` Mike Frysinger
2017-02-20 13:57   ` Djalal Harouni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170216174750.4995-1-asarai@suse.de \
    --to=asarai@suse.de \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=cyphar@cyphar.com \
    --cc=dev@opencontainers.org \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®