lkml.org 
[lkml]   [2021]   [May]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH 2/3] getgroups: hide unknown groups
On Mon, May 10, 2021 at 03:00:10PM +0200, Giuseppe Scrivano wrote:
> do not copy to userspace the groups that are not known in the
> namespace.
>
> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Reviewed-by: Serge Hallyn <serge@hallyn.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

\
 
 \ /
  Last update: 2021-05-15 03:22    [W:0.257 / U:0.352 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site