Re: [PATCH] mod_ldap: fix apr_ldap_rebind_remove() use

2020-06-16 Thread Eric Covener
On Tue, Jun 16, 2020 at 10:40 AM Joe Orton wrote: > > On Thu, May 07, 2020 at 09:50:26AM -0400, Eric Covener wrote: > > On Thu, May 7, 2020 at 9:39 AM Joe Orton wrote: > > > Better question... or stupider question? For "modern" OpenLDAP where > > > ldap_set_rebind_proc takes a void *, this linke

Re: [PATCH] mod_ldap: fix apr_ldap_rebind_remove() use

2020-06-16 Thread Joe Orton
On Thu, May 07, 2020 at 09:50:26AM -0400, Eric Covener wrote: > On Thu, May 7, 2020 at 9:39 AM Joe Orton wrote: > > Better question... or stupider question? For "modern" OpenLDAP where > > ldap_set_rebind_proc takes a void *, this linked list cache is > > completely redundant and you can "simply"

Re: [PATCH] mod_ldap: fix apr_ldap_rebind_remove() use

2020-05-07 Thread Eric Covener
On Thu, May 7, 2020 at 9:39 AM Joe Orton wrote: > > On Thu, May 07, 2020 at 08:06:00AM -0400, Eric Covener wrote: > > On Thu, May 7, 2020 at 2:04 AM Ruediger Pluem wrote: > > > Not looked at the problem further, but there is now a PR for this: > > > > > > https://bz.apache.org/bugzilla/show_bug.c

Re: [PATCH] mod_ldap: fix apr_ldap_rebind_remove() use

2020-05-07 Thread Joe Orton
On Thu, May 07, 2020 at 08:06:00AM -0400, Eric Covener wrote: > On Thu, May 7, 2020 at 2:04 AM Ruediger Pluem wrote: > > Not looked at the problem further, but there is now a PR for this: > > > > https://bz.apache.org/bugzilla/show_bug.cgi?id=64414 > > > > Maybe this revives the discussion :-) >

Re: [PATCH] mod_ldap: fix apr_ldap_rebind_remove() use

2020-05-07 Thread Eric Covener
> > And can we still do an ldap_unbind_s(ldc->ldap); if we did an > > apr_ldap_rebind_remove(ldc->ldap); before? > > This is a good point. For the IBM/Tivoli SDK the callback can be > called a 2nd time as a cleanup (in the IBM SDK terms, not APR). This > will trigger a full/failing search of the

Re: [PATCH] mod_ldap: fix apr_ldap_rebind_remove() use

2020-05-07 Thread Eric Covener
On Wed, Apr 29, 2020 at 2:43 PM Ruediger Pluem wrote: > > > > On 4/29/20 5:02 PM, Eric Covener wrote: > > On Wed, Apr 29, 2020 at 9:29 AM Joe Orton 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

Re: [PATCH] mod_ldap: fix apr_ldap_rebind_remove() use

2020-05-07 Thread Eric Covener
> My hope is that the httpd-only fix will eliminate the leaks and the > locking will not be too much relative to ldap costs, especially w/ the > caching in mod_ldap. Above is wrong since there is really no leak currently as Joe pointed out, the apr_pool_clear is going to remove the entry with its

Re: [PATCH] mod_ldap: fix apr_ldap_rebind_remove() use

2020-05-07 Thread Eric Covener
On Thu, May 7, 2020 at 2:04 AM Ruediger Pluem wrote: > > > > On 4/30/20 11:34 AM, Joe Orton wrote: > > On Wed, Apr 29, 2020 at 03:09:28PM -0400, Eric Covener wrote: > >> On Wed, Apr 29, 2020 at 2:43 PM Ruediger Pluem wrote: > >>> Hm. Don't we need to do the apr_pool_clear(ldc->rebind_pool); in an

Re: [PATCH] mod_ldap: fix apr_ldap_rebind_remove() use

2020-05-06 Thread Ruediger Pluem
On 4/30/20 11:34 AM, Joe Orton wrote: > On Wed, Apr 29, 2020 at 03:09:28PM -0400, Eric Covener wrote: >> On Wed, Apr 29, 2020 at 2:43 PM Ruediger Pluem wrote: >>> Hm. Don't we need to do the apr_pool_clear(ldc->rebind_pool); in any case >>> if ldc->ChaseReferrals == AP_LDAP_CHASEREFERRALS_ON o

Re: [PATCH] mod_ldap: fix apr_ldap_rebind_remove() use

2020-04-30 Thread Joe Orton
On Wed, Apr 29, 2020 at 03:09:28PM -0400, Eric Covener wrote: > On Wed, Apr 29, 2020 at 2:43 PM Ruediger Pluem wrote: > > 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->lda

Re: [PATCH] mod_ldap: fix apr_ldap_rebind_remove() use

2020-04-29 Thread Eric Covener
On Wed, Apr 29, 2020 at 2:43 PM Ruediger Pluem wrote: > > > > On 4/29/20 5:02 PM, Eric Covener wrote: > > On Wed, Apr 29, 2020 at 9:29 AM Joe Orton 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

Re: [PATCH] mod_ldap: fix apr_ldap_rebind_remove() use

2020-04-29 Thread Ruediger Pluem
On 4/29/20 5:02 PM, Eric Covener wrote: > On Wed, Apr 29, 2020 at 9:29 AM Joe Orton 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 >

Re: [PATCH] mod_ldap: fix apr_ldap_rebind_remove() use

2020-04-29 Thread Eric Covener
On Wed, Apr 29, 2020 at 9:29 AM Joe Orton 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 it

[PATCH] mod_ldap: fix apr_ldap_rebind_remove() use

2020-04-29 Thread Joe Orton
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 m