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) {
}