Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Giuseppe Scrivano <gscrivan@redhat.com>
To:     linux-kernel@vger.kernel.org
Cc:     serge@hallyn.com, dwalsh@redhat.com, christian.brauner@ubuntu.com,
        ebiederm@xmission.com
Subject: [RFC PATCH 2/3] getgroups: hide unknown groups
Date:   Mon, 10 May 2021 15:00:10 +0200
Message-Id: <20210510130011.1441834-3-gscrivan@redhat.com>
In-Reply-To: <20210510130011.1441834-1-gscrivan@redhat.com>
References: <20210510130011.1441834-1-gscrivan@redhat.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

do not copy to userspace the groups that are not known in the
namespace.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
---
 kernel/groups.c | 40 +++++++++++++++++++--------------------
 kernel/uid16.c  | 50 ++++++++++++++++++++++++-------------------------
 2 files changed, 44 insertions(+), 46 deletions(-)

diff --git a/kernel/groups.c b/kernel/groups.c
index f0c3b49da19e..97fa9faa7813 100644
--- a/kernel/groups.c
+++ b/kernel/groups.c
@@ -35,19 +35,30 @@ EXPORT_SYMBOL(groups_free);
 
 /* export the group_info to a user-space array */
 static int groups_to_user(gid_t __user *grouplist,
-			  const struct group_info *group_info)
+			  const struct group_info *group_info,
+			  int gidsetsize)
 {
 	struct user_namespace *user_ns = current_user_ns();
-	int i;
 	unsigned int count = group_info->ngroups;
+	int i, ngroups = 0;
 
 	for (i = 0; i < count; i++) {
 		gid_t gid;
-		gid = from_kgid_munged(user_ns, group_info->gid[i]);
-		if (put_user(gid, grouplist+i))
-			return -EFAULT;
+		gid = from_kgid(user_ns, group_info->gid[i]);
+		if (gid == (gid_t)-1) {
+			if (user_ns->flags & USERNS_SETGROUPS_SHADOW)
+				continue;
+			gid = overflowgid;
+		}
+		if (gidsetsize) {
+			if (ngroups == gidsetsize)
+				return -EINVAL;
+			if (put_user(gid, grouplist + ngroups))
+				return -EFAULT;
+		}
+		ngroups++;
 	}
-	return 0;
+	return ngroups;
 }
 
 /* fill a group_info from a user-space array - it must be allocated already */
@@ -146,26 +157,13 @@ EXPORT_SYMBOL(set_current_groups);
 
 SYSCALL_DEFINE2(getgroups, int, gidsetsize, gid_t __user *, grouplist)
 {
+	/* no need to grab task_lock here; it cannot change */
 	const struct cred *cred = current_cred();
-	int i;
 
 	if (gidsetsize < 0)
 		return -EINVAL;
 
-	/* no need to grab task_lock here; it cannot change */
-	i = cred->group_info->ngroups;
-	if (gidsetsize) {
-		if (i > gidsetsize) {
-			i = -EINVAL;
-			goto out;
-		}
-		if (groups_to_user(grouplist, cred->group_info)) {
-			i = -EFAULT;
-			goto out;
-		}
-	}
-out:
-	return i;
+	return groups_to_user(grouplist, cred->group_info, gidsetsize);
 }
 
 bool may_setgroups(struct group_info **shadowed_groups)
diff --git a/kernel/uid16.c b/kernel/uid16.c
index cb1110f083ce..6f2dc793b5f8 100644
--- a/kernel/uid16.c
+++ b/kernel/uid16.c
@@ -112,21 +112,33 @@ SYSCALL_DEFINE1(setfsgid16, old_gid_t, gid)
 }
 
 static int groups16_to_user(old_gid_t __user *grouplist,
-    struct group_info *group_info)
+			    struct group_info *group_info,
+			    int gidsetsize)
 {
 	struct user_namespace *user_ns = current_user_ns();
-	int i;
-	old_gid_t group;
-	kgid_t kgid;
+	unsigned int count = group_info->ngroups;
+	int i, ngroups = 0;
 
-	for (i = 0; i < group_info->ngroups; i++) {
-		kgid = group_info->gid[i];
-		group = high2lowgid(from_kgid_munged(user_ns, kgid));
-		if (put_user(group, grouplist+i))
-			return -EFAULT;
+	for (i = 0; i < count; i++) {
+		gid_t gid;
+		old_gid_t group;
+
+		gid = from_kgid(user_ns, group_info->gid[i]);
+		if (gid == (gid_t)-1) {
+			if (user_ns->flags & USERNS_SETGROUPS_SHADOW)
+				continue;
+			gid = overflowgid;
+		}
+		group = high2lowgid(gid);
+		if (gidsetsize) {
+			if (ngroups == gidsetsize)
+				return -EINVAL;
+			if (put_user(group, grouplist + ngroups))
+				return -EFAULT;
+		}
+		ngroups++;
 	}
-
-	return 0;
+	return ngroups;
 }
 
 static int groups16_from_user(struct group_info *group_info,
@@ -153,25 +165,13 @@ static int groups16_from_user(struct group_info *group_info,
 
 SYSCALL_DEFINE2(getgroups16, int, gidsetsize, old_gid_t __user *, grouplist)
 {
+	/* no need to grab task_lock here; it cannot change */
 	const struct cred *cred = current_cred();
-	int i;
 
 	if (gidsetsize < 0)
 		return -EINVAL;
 
-	i = cred->group_info->ngroups;
-	if (gidsetsize) {
-		if (i > gidsetsize) {
-			i = -EINVAL;
-			goto out;
-		}
-		if (groups16_to_user(grouplist, cred->group_info)) {
-			i = -EFAULT;
-			goto out;
-		}
-	}
-out:
-	return i;
+	return groups16_to_user(grouplist, cred->group_info, gidsetsize);
 }
 
 SYSCALL_DEFINE2(setgroups16, int, gidsetsize, old_gid_t __user *, grouplist)
-- 
2.31.1