On Sun, Jun 16, 2013 at 12:29:26PM +0200, Jakub Hrozek wrote:
> On Fri, Jun 14, 2013 at 03:08:28PM +0200, Pavel Březina wrote:
>>
https://fedorahosted.org/sssd/ticket/1894
>>
>> Reproducer:
>> group-1 -> group-2 -> enough groups that it triggers AD range retrieval
>> getent group group-1
>>
>> I used 3000 groups and 1500 users.
>>
>> There is only one member in group-1 so it is processed with single step. AD
>> will return maximum 1500 values per attribute by default, so we allocate
>> space for 1500 groups. Then ASQ is called and returns 4500 members so we end
>> up writing after allocated space.
>>
>
> Nice catch!
>
>> Normally, we split the members to users, groups and unknown objects and
>> allocate space for groups + unknown. But I don't think it is necessary to do
>> the split here again. We allocate potentially more space than required and
>> then shrink it to the correct size.
>
> I tend to agree, unless there would be a very long running request with
> a huge number of members. But then again, the member of the array we
> allocate are sizeof(struct sysdb_attrs *) so it's not a big deal.
>
> Just one question below..
>
>> --- a/src/providers/ldap/sdap_async_nested_groups.c
>> +++ b/src/providers/ldap/sdap_async_nested_groups.c
>> @@ -2048,6 +2048,18 @@ sdap_nested_group_deref_direct_process(struct tevent_req
*subreq)
>> DEBUG(SSSDBG_TRACE_INTERNAL, ("Received %d dereference results,
"
>> "about to process them\n", num_entries));
>>
>> + if (num_entries != members->num_values) {
>
> Why does this condition say "!=" and not "<=" ? Can we ever
allocate to
Sorry, the way I propose the change, we'd also have to reverse the
operands and compare with < not <=:
if (members->num_values < num_entries) {
}
You are correct sir. Thanks.