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
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to