Re: [SSSD] [PATCH] Keep deref controls until the whole request is finished
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