Re: [PATCH 18/43] userns: Convert group_info values from gid_t to kgid_t.

Eric W. Biederman
2012-04-20T23:01:35Z

"Serge E. Hallyn" <serge@hallyn.com> writes:

> Quoting Eric W. Beiderman (ebiederm@xmission.com):
>> From: Eric W. Biederman <ebiederm@xmission.com>
>>
>> As a first step to converting struct cred to be all kuid_t and kgid_t
>> values convert the group values stored in group_info to always be
>> kgid_t values. Unless user namespaces are used this change should
>> have no effect.
>>
>> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
>> ---
>> arch/s390/kernel/compat_linux.c | 13 ++++++++-
>> fs/nfsd/auth.c | 5 ++-
>> fs/proc/array.c | 5 +++-
>> include/linux/cred.h | 9 ++++---
>> kernel/groups.c | 48 +++++++++++++++++++-----------------
>> kernel/uid16.c | 14 +++++++++-
>> net/ipv4/ping.c | 11 ++++++--
>> net/sunrpc/auth_generic.c | 4 +-
>> net/sunrpc/auth_gss/svcauth_gss.c | 7 ++++-
>> net/sunrpc/auth_unix.c | 15 ++++++++---
>> net/sunrpc/svcauth_unix.c | 18 ++++++++++---
>> security/keys/permission.c | 3 +-
>> 12 files changed, 103 insertions(+), 49 deletions(-)
>>
>> diff --git a/arch/s390/kernel/compat_linux.c b/arch/s390/kernel/compat_linux.c
>> index ab64bdb..5baac18 100644
>> --- a/arch/s390/kernel/compat_linux.c
>> +++ b/arch/s390/kernel/compat_linux.c
>> @@ -173,11 +173,14 @@ asmlinkage long sys32_setfsgid16(u16 gid)
>>
>> static int groups16_to_user(u16 __user *grouplist, struct group_info *group_info)
>> {
>> + struct user_namespace *user_ns = current_user_ns();
>> int i;
>> u16 group;
>> + kgid_t kgid;
>>
>> for (i = 0; i < group_info->ngroups; i++) {
>> - group = (u16)GROUP_AT(group_info, i);
>> + kgid = GROUP_AT(group_info, i);
>> + group = (u16)from_kgid_munged(user_ns, kgid);
>> if (put_user(group, grouplist+i))
>> return -EFAULT;
>> }
>> @@ -187,13 +190,19 @@ static int groups16_to_user(u16 __user *grouplist, struct group_info *group_info
>>
>> static int groups16_from_user(struct group_info *group_info, u16 __user *grouplist)
>> {
>> + struct user_namespace *user_ns = current_user_ns();
>> int i;
>> u16 group;
>
> need
>
> kgid_t kgid;
>
> here

Doh! Thank you.

Now I am wondering why s390 has it's own copy of this function instead
of using the version in uid16.c.

>> for (i = 0; i < group_info->ngroups; i++) {
>> if (get_user(group, grouplist+i))
>> return -EFAULT;
>> - GROUP_AT(group_info, i) = (gid_t)group;
>> +
>> + kgid = make_kgid(user_ns, (gid_t)group);
>> + if (!gid_valid(kgid))
>> + return -EINVAL;
>> +
>> + GROUP_AT(group_info, i) = kgid;
>> }
>>
>> return 0;

Eric