[SSSD] [PATCH] fix early free of sdap_handle

Eugene Indenbom eindenbom at gmail.com
Wed Apr 14 06:48:28 UTC 2010


Hi Simo,

On 04/13/2010 08:51 PM, Simo Sorce wrote:
> This patch attempts to address the problem Eugene found with
> sdap_handle_release()
>
>    
I have checked the suggested patch and found several problems:

1. providers/ipa/ipa_access.c still lacks failover retries

2. Why number of retries is 2? It is only enough for one server. If we 
have failover set up we need more retries (at least 1 per server + 1 for 
stale cached connection)

3. My patch contained an additional fix to sdap_handle_release. Before 
calling op callbacks LDAP callback data was destroyed:

commit: sssd-1_1_1
line 102:        talloc_zfree(sh->conncb->lc_arg);

and later LDAP handle was unbound:
line 116:        ldap_unbind_ext(sh->ldap, NULL, NULL);

But during unbind the LDAP callbacks are called. And sssd callback 
(sdap_ldap_connect_callback_del) accesses zero pointer (lc_arg).

So before freeing sh->conncb->lc_arg it is necessary to call:
                 /* remove callback first */
                 ldap_get_option(sh->ldap, LDAP_OPT_CONNECT_CB, sh->conncb);

Actually I wonder how does it work for you without this fix? Probably 
you have never tried to close a connection with still open file descriptor.

4. The suggested patch is a workaround. It relies on the agreement that 
all _send functions will be called from top level event context, not 
from subreq callback context. This is hard to guarantee.

I have attached to this e-mail a version of my patch addressing the same 
problem that applies directly on top of sssd 1.1.1.

I think its better to use my solution as:

1. It provides a real fix for the problem rather than workaround
2. The complexity of patches is alike (5 files, 213 versus 138 changed 
lines)
3. The real changes in my patch are localized to sdap.h, 
sdap_async_private.h, sdap_handle_create, sdap_handle_release
4. Other changes are cosmetic and related to changes in structure 
definition and therefore can be easily checked by peer review

Regards, Eugene
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-Fixed-recursive-sdap_handle-disconnect-sequence-from.patch
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20100414/a358c431/attachment.ksh>


More information about the sssd-devel mailing list