[Freeipa-devel] [PATCH] Fix race condition leading to segfaults

[Sumit Bose]

On Thu, Jul 23, 2009 at 04:18:15PM -0400, Simo Sorce wrote:
> 
> Sumit found out that ldap auth would segfault from time to time.
> 
> The problem was the way ldap_result() works you don't know how many
> results are in the pipe so you have to keep polling, once the fde fired,
> until you get back a 0.
> 
> The problem although, was that once a result was received the calling
> function, upon getting called by op->callback(), could decide to free
> the sdap_handle and all dependent memory like it happens in
> sdap_pam_auth_done() which is called through a chain of callbacks from
> sdap_process_message()
> 
> The following patch addresses the problem using, temporarily a timer
> event to schedule any other call only after the callback is finished.
> The events are appended on the sdap_handle, so that if the function
> being called actually frees it, all pending events are also freed
> without risk.

No more segfaults (so far :-)

But I think the following piece of code might be an example where gcc
might optimize away a check, because the pointers to check
(sh->ldap) are already referenced in the debug
statement. The glibc printf which is used by DEBUG can handle NULL
pointers and will not segfault here and if the check is optimized away
ldap_result won't like sh->ldap being NULL.

> +
> +    DEBUG(8, ("Trace: sh[%p], connected[%d], ops[%p], fde[%p], ldap[%p]\n",
> +              sh, (int)sh->connected, sh->ops, sh->fde, sh->ldap));
> +
> +    if (!sh->connected || !sh->ldap) {
> +        DEBUG(2, ("ERROR: LDAP connection is not connected!\n"));
> +        return;
> +    }
> +
> +    ret = ldap_result(sh->ldap, LDAP_RES_ANY, 0, &no_timeout, &msg);


bye,
Sumit