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;
+ }
+ }
+