On 4/29/20 5:02 PM, Eric Covener wrote:
> On Wed, Apr 29, 2020 at 9:29 AM Joe Orton <[email protected]> wrote:
>>
>> In uldap_connection_unbind, apr_ldap_rebind_remove() is always passed
>> NULL since ldc->ldap is either NULL on entry or is set to NULL. This
>> looks safe, but seems like an expensive noop since
>> apr_ldap_rebind_remove() acquires a mutex and iterates a linked list
>> trying to find a rebind reference matching NULL (I assume always
>> failing).
>>
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap.c?revision=1876599&view=markup#l222
>>
>> Can this be removed or should it be moved up so it's not a noop? I've
>> not tried to reproduce problems in an LDAP config with referrals.
>>
>> (Credit to Abhishek and Angel who saw apr_ldap_rebind_remove() spinning
>> and debugged this)
>>
>> Index: modules/ldap/util_ldap.c
>> ===================================================================
>> --- modules/ldap/util_ldap.c (revision 1877157)
>> +++ modules/ldap/util_ldap.c (working copy)
>> @@ -225,6 +225,12 @@
>>
>> if (ldc) {
>> if (ldc->ldap) {
>> + /* forget the rebind info for this conn */
>> + if (ldc->ChaseReferrals == AP_LDAP_CHASEREFERRALS_ON) {
>> + apr_ldap_rebind_remove(ldc->ldap);
>> + apr_pool_clear(ldc->rebind_pool);
>> + }
>> +
>> if (ldc->r) {
>> ap_log_rerror(APLOG_MARK, APLOG_TRACE5, 0, ldc->r, "LDC %pp
>> unbind", ldc);
>> }
>> @@ -233,11 +239,6 @@
>> }
>> ldc->bound = 0;
>>
>> - /* forget the rebind info for this conn */
>> - if (ldc->ChaseReferrals == AP_LDAP_CHASEREFERRALS_ON) {
>> - apr_ldap_rebind_remove(ldc->ldap);
>> - apr_pool_clear(ldc->rebind_pool);
>> - }
>> }
>
> +1 to moving up
Hm. Don't we need to do the apr_pool_clear(ldc->rebind_pool); in any case if
ldc->ChaseReferrals == AP_LDAP_CHASEREFERRALS_ON or
does this only makes sense if ldc->ldap != NULL?
And can we still do an ldap_unbind_s(ldc->ldap); if we did an
apr_ldap_rebind_remove(ldc->ldap); before?
Regards
Rüdiger
>
