svn commit: r332559 - head/usr.sbin/mountd

Andriy Gapon avg at FreeBSD.org
Mon Apr 16 11:12:42 UTC 2018


On 16/04/2018 13:56, Konstantin Belousov wrote:
> On Mon, Apr 16, 2018 at 09:17:36AM +0000, Andriy Gapon wrote:
>> Author: avg
>> Date: Mon Apr 16 09:17:36 2018
>> New Revision: 332559
>> URL: https://svnweb.freebsd.org/changeset/base/332559
>>
>> Log:
>>   mountd: fix a crash when getgrouplist reports too many groups
>>   
>>   Previously the code only warned about the condition and then happily
>>   proceeded to use the too large value resulting in the array
>>   out-of-bounds access.
>>   
>>   Obtained from:	Panzura (Chuanbo Zheng)
>>   MFC after:	10 days
>>   Sponsored by:	Panzura
>>
>> Modified:
>>   head/usr.sbin/mountd/mountd.c
>>
>> Modified: head/usr.sbin/mountd/mountd.c
>> ==============================================================================
>> --- head/usr.sbin/mountd/mountd.c	Mon Apr 16 08:41:44 2018	(r332558)
>> +++ head/usr.sbin/mountd/mountd.c	Mon Apr 16 09:17:36 2018	(r332559)
>> @@ -2915,8 +2915,11 @@ parsecred(char *namelist, struct xucred *cr)
>>  		}
>>  		cr->cr_uid = pw->pw_uid;
>>  		ngroups = XU_NGROUPS + 1;
>> -		if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups))
>> +		if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups)) {
>>  			syslog(LOG_ERR, "too many groups");
>> +			ngroups = XU_NGROUPS + 1;
> Why XU_NGROUPS and not the value of sysctl("kern.ngroups") ?

Two reasons:

1. it's what the code already used
2. the groups are placed into struct xucred and later that struct is passed to
kernel, so in my opinion it's xucred that defines the limit in this case


-- 
Andriy Gapon


More information about the svn-src-head mailing list