Full_Name: Ian Puleston Version: 2.4.40 OS: VxWorks URL: ftp://ftp.openldap.org/incoming/ Submission from: (NULL) (204.118.31.9) If connect and delete callbacks are registered via LDAP_OPT_CONNECT_CB, then the lc_add callback is called when the TCP connects, but if TLS then fails to connect the lc_del does not get called. If these callbacks are doing something like allocating/freeing resources this can lead to a resource leak. When I am seeing this, the sequence of calls leading to the lc_add callback is as follows: ldap_new_connection -> ldap_int_open_connection -> ldap_connect_to_host -> ldap_int_connect_cbs() Then if ldap_connect_to_host returned 0 for synchronous success this call happens: ldap_new_connection -> ldap_int_open_connection -> ldap_int_tls_start And if that fails and returns something other than LDAP_SUCCESS then ldap_int_open_connection exits. So we got a call to lc_add but no corresponding call to lc_del. And note that there is not call to ldap_free_connection, presumably because ldap-new-connection did not succeed, hence the lc_del callback does not get called from there. In my case when I see this it is on a referral with the above calls made from ldap_chase_v3referrals, hence it is happening synchronously. Whether the same would happen with an asynchronous connect I don't know. I do have a working fix which is to make the lc_del callbacks directly from ldap_int_open_connection if ldap_int_tls_start fails. I will supply a patch for that shortly.
I have uploaded the patch to ftp.openldap.org/incoming as ian-puleston-170825.patch, and included it below. The code that it adds is pretty much copied from ldap_free_connection: diff --git libraries/libldap/open.c libraries/libldap/open.c index 11564f9..251289d 100644 --- libraries/libldap/open.c +++ libraries/libldap/open.c @@ -450,6 +450,31 @@ ldap_int_open_connection( --conn->lconn_refcnt; if (rc != LDAP_SUCCESS) { + /* report the failure to the connection callbacks */ + { + struct ldapoptions *lo; + ldaplist *ll; + ldap_conncb *cb; + + lo = &ld->ld_options; + LDAP_MUTEX_LOCK( &lo->ldo_mutex ); + if ( lo->ldo_conn_cbs ) { + for ( ll=lo->ldo_conn_cbs; ll; ll=ll->ll_next ) { + cb = ll->ll_data; + cb->lc_del( ld, conn->lconn_sb, cb ); + } + } + LDAP_MUTEX_UNLOCK( &lo->ldo_mutex ); + lo = LDAP_INT_GLOBAL_OPT(); + LDAP_MUTEX_LOCK( &lo->ldo_mutex ); + if ( lo->ldo_conn_cbs ) { + for ( ll=lo->ldo_conn_cbs; ll; ll=ll->ll_next ) { + cb = ll->ll_data; + cb->lc_del( ld, conn->lconn_sb, cb ); + } + } + LDAP_MUTEX_UNLOCK( &lo->ldo_mutex ); + } return -1; } }
changed notes changed state Open to Test moved from Incoming to Software Bugs
ipuleston@SonicWALL.com wrote: > I have uploaded the patch to ftp.openldap.org/incoming as ian-puleston-1708= > 25.patch, and included it below. > > The code that it adds is pretty much copied from ldap_free_connection: Thanks for the report, added in master. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
changed notes
changed notes changed state Test to Release
fixed in master fixed in RE24 (2.4.46)
changed notes changed state Release to Closed