Re: [Freeipa-devel] [PATCH] Fix race condition leading to segfaults
On Fri, 2009-07-24 at 15:52 +0200, Sumit Bose wrote: > > After a discussion on irc Simo and I came to the conclusion that the > conditional code in DEBUG would prevent gcc from optimizing away the > check. > > ACK pushed. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fix race condition leading to segfaults
On Fri, Jul 24, 2009 at 10:15:25AM +0200, Sumit Bose wrote:
> 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);
>
>
After a discussion on irc Simo and I came to the conclusion that the
conditional code in DEBUG would prevent gcc from optimizing away the
check.
ACK
bye,
Sumit
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fix race condition leading to segfaults
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
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
