Re: [SSSD] [PATCH] LDAP: Fix crash when processing nested groups
On (08/08/13 00:50), Jakub Hrozek wrote:
On Wed, Aug 07, 2013 at 11:36:37PM +0200, Pavel Březina wrote:
> On 08/07/2013 11:59 AM, Jakub Hrozek wrote:
> >On Wed, Aug 07, 2013 at 09:53:13AM +0200, Pavel Březina wrote:
> >>On 08/06/2013 09:25 PM, Jakub Hrozek wrote:
> >>>https://fedorahosted.org/sssd/ticket/1932
> >>>
> >>>There is a rather strange workaround in the nested groups processing
> >>>code that calls tevent_req_post outside _send(). However, it broke in
> >>>certain situations where the tevent_req_call resulted in req being
freed,
> >>>which freed state by extension and then the subsequent _post call was a
> >>>use-after-free. This patch saves the two variables used outside state so
> >>>that it's safe to use them even after the callback.
> >>
> >>Hi,
> >>great catch! Sometimes I wish tevent_req_done() would schedule
> >>immediate event, instead of firing the callback directly.
> >>
> >>Nack to the patch but ack to the solution. The same code is also in
> >>sdap_nested_group_lookup_user(). Can you fix it as well? It maybe
> >>can't be triggered, but who knows...
> >
> >I think it can be triggered if all users are out of all search bases.
> >New patch is attached.
>
> Ack.
Pushed to sssd-1-9
>
> BTW there was lately several use-after-free issues found by users.
> We should start testing our patches with TALLOC_FREE_FILL=0 - this
> will overwrite freed memory with zeros on talloc_free() so if there
> is anything like that in covered code, we will hit that.
Yep :-)
We should also be using MALLOC_PERTURB, which does the same thing for
mallocated memory, making it easier to detect uninitialized access to
heap memory.
Can you add a note about TALLOC_FREE_FILL to
https://fedorahosted.org/sssd/wiki/DevelTips ?
+1
but I would use different value than 0, because it will be impossible
to distinguish between zero initialized memory and released memory (use after
free)
I would prefer very big value for example 0xFD (FreeD memory)
LS