[SSSD] nested groups: allocate more space if deref returns more members

Jakub Hrozek jhrozek at redhat.com
Sun Jun 16 10:29:26 UTC 2013


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
fewer entries? (We can with the other realloc).

> +        /* Dereference returned more values than obtained earlier. We need
> +         * to adjust group array size. */
> +        state->nested_groups = talloc_realloc(state, state->nested_groups,
> +                                              struct sysdb_attrs *,
> +                                              num_entries);
> +        if (state->nested_groups == NULL) {
> +            ret = ENOMEM;
> +            goto done;
> +        }
> +    }
> +



More information about the sssd-devel mailing list