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

[ebiederm@xmission.com (Eric W. Biederman)]

"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