[CRIU] [PATCH] security: check additional groups

Ruslan Kuprieiev kupruser at gmail.com
Wed Jul 2 05:17:27 PDT 2014


On 02.07.2014 14:58, Pavel Emelyanov wrote:
>> -void restrict_uid(unsigned int uid, unsigned int gid)
>> +int restrict_uid(unsigned int uid, unsigned int gid)
>>   {
>> -	pr_info("Restrict C/R with %u:%u uid\n", uid, gid);
>> +	struct passwd *pwd;
>> +	unsigned int buf[NGROUPS_MAX];
>> +	int nbuf;
>> +
>> +	pr_info("Restrict C/R with %u:%u uid:gid\n", uid, gid);
>>   	cr_uid = uid;
>>   	cr_gid = gid;
>> +
>> +	pwd = getpwuid(uid);
>> +	if (!pwd) {
>> +		pr_perror("Can't get password file entry");
>> +		return -1;
>> +	}
>> +
>> +	nbuf = NGROUPS_MAX;
>> +	if (getgrouplist(pwd->pw_name, pwd->pw_gid, buf, &nbuf) < 0) {
> There's no need in getting the caller's groups. It's enough just
> to check the group it currently lives in.

User's current group may not match all gids of the process. Just as it 
was with
that game. When user's group was 1000, but process had (1000,20,20).

And we should check that user isn't restoring process which has more groups
than user already has, shouldn't we?

>> +		pr_perror("Can't get group list");
>> +		return -1;
>> +	}
>> +
>> +	cr_ngroups = nbuf;
>> +	cr_groups = xmalloc(cr_ngroups*sizeof(*cr_groups));
>> +	if (!cr_groups)
>> +		return -1;
>> +
>> +	memcpy(cr_groups, buf, cr_ngroups*sizeof(*cr_groups));
>> +
>> +	return 0;
>>   }



More information about the CRIU mailing list