[SSSD] [PATCH] Keep deref controls until the whole request is finished

Simo Sorce simo at redhat.com
Tue Sep 6 18:40:48 UTC 2011


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




More information about the sssd-devel mailing list