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.
Given Jakub is on vacation, Stephen asked me to handle this one.
I have fixed this minor issue and pushed to master and sssd-1-6
Simo.
--
Simo Sorce * Red Hat, Inc * New York