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

Jakub Hrozek jhrozek at redhat.com
Mon Jun 17 07:49:12 UTC 2013


On Mon, Jun 17, 2013 at 09:44:40AM +0200, Pavel Březina wrote:
> On 06/16/2013 12:52 PM, Jakub Hrozek wrote:
> >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.
> 

Ack



More information about the sssd-devel mailing list