ipuleston@SonicWALL.com wrote:

The patch that I submitted above has a memory leak due not freeing "ber"

when returning error LDAP_X_CONNECTING from function ldap_send_initial_request(). I have uploaded a new version of the patch with this fixed to ftp.openldap.org/incoming as ian-puleston-110412.patch.2 (note the .2 on the end - it was a 2nd attempt after I forgot to specify ASCII mode).

Unfortunately the patch was made against 2.4.23 and not against HEAD. It does not apply cleanly to current git. Patches must always be submitted against the development code, not against released code.

I'm pretty sure that this chunk can never do anything useful.

diff -u libldap.orig/request.c libldap/request.c --- libldap.orig/request.c 2010-06-10 10:39:48.000000000 -0700 +++ libldap/request.c 2011-04-12 12:11:20.769166100 -0700 @@ -446,7 +462,21 @@ lc->lconn_server = ldap_url_dup( srv ); }

- lc->lconn_status = async ? LDAP_CONNST_CONNECTING : LDAP_CONNST_CONNECTED; + if ( async ) { +#ifdef HAVE_TLS + void *ssl = NULL; + ber_sockbuf_ctrl( lc->lconn_sb, LBER_SB_OPT_GET_SSL, (void *)&ssl ); + if ( ssl ) { + /* the connect finished and tls was started */ + lc->lconn_status = LDAP_CONNST_CONNECTED; + } else +#endif + { + lc->lconn_status = LDAP_CONNST_CONNECTING; + } + } else { + lc->lconn_status = LDAP_CONNST_CONNECTED; + } #ifdef LDAP_R_COMPILE ldap_pvt_thread_mutex_lock( &ld->ld_conn_mutex ); #endif

The connection has just been created, asynchronously, so there's no way that the TLS layer was already started when it got here. Also, even if this was valid, it obviously would only apply for OPT_X_TLS_HARD || ldaps, but you haven't checked that condition here. Ultimately it appears that this test is a no-op.

Also, I suggest that you only check for CONNST_CONNECTING in the callers, and do the TLS check in the check function, like this:

+int +ldap_int_check_async_open( LDAP *ld, LDAPConn *lc, LDAPURLDesc *srv, ber_socket_t sd ) +{ + struct timeval tv = { 0 }; + int rc; + + rc = ldap_int_poll( ld, sd, &tv ); + switch ( rc ) { + case 0: + /* now ready to start tls */ + lc->lconn_status = LDAP_CONNST_CONNECTED; + break; + + default: + rc = -1; + return rc; + case -2: + /* connect not completed yet */ + ld->ld_errno = LDAP_X_CONNECTING; + return rc; + } + +#ifdef HAVE_TLS + if ( ld->ld_options.ldo_tls_mode == LDAP_OPT_X_TLS_HARD || + !strcmp( srv->lud_scheme, "ldaps" )) { + + ++lc->lconn_refcnt; /* avoid premature free */ + + rc = ldap_int_tls_start( ld, lc, srv ); + + --lc->lconn_refcnt; + } +#endif + return rc; +}