On Tue, 2011-09-06 at 15:18 +0200, Jakub Hrozek wrote: > On Tue, Sep 06, 2011 at 08:15:16AM -0400, Simo Sorce wrote: > > On Tue, 2011-09-06 at 12:43 +0200, Jakub Hrozek wrote: > > > > > > > > > http://fedorahosted.org/sssd/ticket/989 > > > > > > John Hodrien found out that when paging is used while dereferencing an > > > entry, sssd_be may segfault on the second page. > > > > > > This was because paging returned the control to sdap_generic_search > > > multiple times but sssd was freeing dereference control after the > > > first > > > search invocation. The subsequend sdap searched accessed memory that > > > was > > > already freed. > > > > > > John confirmed off-list that this patch fixed his issue. > > > > > > I was also considering copying the controls into the search request, > > > but > > > it seemed like a pointless allocation. > > > > I am not sure freeing explicitly in the _done() function is bullet > > proof. There are cases where we might kill the operation without going > > through the _done() function. > > You should rather allocate the ctrls array using talloc_zero(), and then > > attach a destructor to free ctrls[0] if it is not NULL. > > > > Good idea, a new patch is attached.
Nack (minor) If sdap_x_deref_create_control() returns an error state->ctrls[0] is undetermined, so the following tallof_free() may call ldap_control_free() on a random address. You should use talloc_zero_array() to make sure the structure elements are initialized to zero, which would also make the following assignment of state->ctrls[1] unnecessary. Simo. -- Simo Sorce * Red Hat, Inc * New York _______________________________________________ sssd-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/sssd-devel
