From 5f07d15c4832a9309c7e3573f56cbf6ec0f5cdfe Mon Sep 17 00:00:00 2001 From: Kurt Zeilenga Date: Tue, 10 Jan 2006 02:07:31 +0000 Subject: [PATCH] Fixed slapd-ldap/meta session reuse issue (ITS#3415) Fixed slapd-monitor thread issue (ITS#3418) --- CHANGES | 4 ++ build/version.var | 2 +- servers/slapd/back-ldap/back-ldap.h | 11 +++++- servers/slapd/back-ldap/bind.c | 54 ++++++++++++++++++-------- servers/slapd/back-meta/bind.c | 2 +- servers/slapd/back-meta/conn.c | 60 ++++++++++++++++++++--------- servers/slapd/back-monitor/thread.c | 10 ++++- 7 files changed, 102 insertions(+), 41 deletions(-) diff --git a/CHANGES b/CHANGES index e46bf63c17..470b8230f5 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,9 @@ OpenLDAP 2.3 Change Log +OpenLDAP 2.3.17 Engineering + Fixed slapd-ldap/meta session reuse issue (ITS#3415) + Fixed slapd-monitor thread issue (ITS#3418) + OpenLDAP 2.3.16 Release Fixed slapd-bdb reindexing via cn=config not noticed issue (ITS#4260) Fixed slapd-monitor connection search crash (ITS#4300) diff --git a/build/version.var b/build/version.var index 76794ba403..59554f5b92 100644 --- a/build/version.var +++ b/build/version.var @@ -15,7 +15,7 @@ ol_package=OpenLDAP ol_major=2 ol_minor=3 -ol_patch=16 +ol_patch=X ol_api_inc=20316 ol_api_current=2 ol_api_revision=4 diff --git a/servers/slapd/back-ldap/back-ldap.h b/servers/slapd/back-ldap/back-ldap.h index 5e243bc3d6..dd16d53d05 100644 --- a/servers/slapd/back-ldap/back-ldap.h +++ b/servers/slapd/back-ldap/back-ldap.h @@ -59,6 +59,7 @@ typedef struct ldapconn_t { #define LDAP_BACK_FCONN_ISBMASK (LDAP_BACK_FCONN_ISBOUND|LDAP_BACK_FCONN_ISANON) #define LDAP_BACK_FCONN_ISPRIV (0x04) #define LDAP_BACK_FCONN_ISTLS (0x08) +#define LDAP_BACK_FCONN_BINDING (0x10) #define LDAP_BACK_CONN_ISBOUND(lc) LDAP_BACK_CONN_ISSET((lc), LDAP_BACK_FCONN_ISBOUND) #define LDAP_BACK_CONN_ISBOUND_SET(lc) LDAP_BACK_CONN_SET((lc), LDAP_BACK_FCONN_ISBOUND) @@ -76,6 +77,9 @@ typedef struct ldapconn_t { #define LDAP_BACK_CONN_ISTLS_SET(lc) LDAP_BACK_CONN_SET((lc), LDAP_BACK_FCONN_ISTLS) #define LDAP_BACK_CONN_ISTLS_CLEAR(lc) LDAP_BACK_CONN_CLEAR((lc), LDAP_BACK_FCONN_ISTLS) #define LDAP_BACK_CONN_ISTLS_CPY(lc, mlc) LDAP_BACK_CONN_CPY((lc), LDAP_BACK_FCONN_ISTLS, (mlc)) +#define LDAP_BACK_CONN_BINDING(lc) LDAP_BACK_CONN_ISSET((lc), LDAP_BACK_FCONN_BINDING) +#define LDAP_BACK_CONN_BINDING_SET(lc) LDAP_BACK_CONN_SET((lc), LDAP_BACK_FCONN_BINDING) +#define LDAP_BACK_CONN_BINDING_CLEAR(lc) LDAP_BACK_CONN_CLEAR((lc), LDAP_BACK_FCONN_BINDING) unsigned lc_refcnt; unsigned lc_flags; @@ -193,7 +197,12 @@ typedef enum ldap_back_send_t { LDAP_BACK_DONTSEND = 0x00, LDAP_BACK_SENDOK = 0x01, LDAP_BACK_SENDERR = 0x02, - LDAP_BACK_SENDRESULT = (LDAP_BACK_SENDOK|LDAP_BACK_SENDERR) + LDAP_BACK_SENDRESULT = (LDAP_BACK_SENDOK|LDAP_BACK_SENDERR), + LDAP_BACK_BINDING = 0x04, + LDAP_BACK_BIND_DONTSEND = (LDAP_BACK_BINDING), + LDAP_BACK_BIND_SOK = (LDAP_BACK_BINDING|LDAP_BACK_SENDOK), + LDAP_BACK_BIND_SERR = (LDAP_BACK_BINDING|LDAP_BACK_SENDERR), + LDAP_BACK_BIND_SRES = (LDAP_BACK_BINDING|LDAP_BACK_SENDRESULT) } ldap_back_send_t; /* define to use asynchronous StartTLS */ diff --git a/servers/slapd/back-ldap/bind.c b/servers/slapd/back-ldap/bind.c index cadcaa8b05..54004ebee4 100644 --- a/servers/slapd/back-ldap/bind.c +++ b/servers/slapd/back-ldap/bind.c @@ -56,7 +56,7 @@ ldap_back_bind( Operation *op, SlapReply *rs ) int rc = 0; ber_int_t msgid; - lc = ldap_back_getconn( op, rs, LDAP_BACK_SENDERR ); + lc = ldap_back_getconn( op, rs, LDAP_BACK_BIND_SERR ); if ( !lc ) { return rs->sr_err; } @@ -468,7 +468,7 @@ ldapconn_t * ldap_back_getconn( Operation *op, SlapReply *rs, ldap_back_send_t sendok ) { ldapinfo_t *li = (ldapinfo_t *)op->o_bd->be_private; - ldapconn_t *lc, + ldapconn_t *lc = NULL, lc_curr = { 0 }; int refcnt = 1; @@ -489,22 +489,34 @@ ldap_back_getconn( Operation *op, SlapReply *rs, ldap_back_send_t sendok ) } } - /* Searches for a ldapconn in the avl tree */ - ldap_pvt_thread_mutex_lock( &li->li_conninfo.lai_mutex ); + /* Explicit Bind requests always get their own conn */ + if ( !( sendok & LDAP_BACK_BINDING ) ) { + /* Searches for a ldapconn in the avl tree */ +retry_lock: + ldap_pvt_thread_mutex_lock( &li->li_conninfo.lai_mutex ); - lc = (ldapconn_t *)avl_find( li->li_conninfo.lai_tree, - (caddr_t)&lc_curr, ldap_back_conn_cmp ); - if ( lc != NULL ) { - refcnt = ++lc->lc_refcnt; + lc = (ldapconn_t *)avl_find( li->li_conninfo.lai_tree, + (caddr_t)&lc_curr, ldap_back_conn_cmp ); + if ( lc != NULL ) { + /* Don't reuse connections while they're still binding */ + if ( LDAP_BACK_CONN_BINDING( lc ) ) { + ldap_pvt_thread_mutex_unlock( &li->li_conninfo.lai_mutex ); + ldap_pvt_thread_yield(); + goto retry_lock; + } + refcnt = ++lc->lc_refcnt; + } + ldap_pvt_thread_mutex_unlock( &li->li_conninfo.lai_mutex ); } - ldap_pvt_thread_mutex_unlock( &li->li_conninfo.lai_mutex ); /* Looks like we didn't get a bind. Open a new session... */ if ( lc == NULL ) { if ( ldap_back_prepare_conn( &lc, op, rs, sendok ) != LDAP_SUCCESS ) { return NULL; } - + if ( sendok & LDAP_BACK_BINDING ) { + LDAP_BACK_CONN_BINDING_SET( lc ); + } lc->lc_conn = lc_curr.lc_conn; ber_dupbv( &lc->lc_local_ndn, &lc_curr.lc_local_ndn ); @@ -618,6 +630,7 @@ ldap_back_release_conn( ldap_pvt_thread_mutex_lock( &li->li_conninfo.lai_mutex ); assert( lc->lc_refcnt > 0 ); lc->lc_refcnt--; + LDAP_BACK_CONN_BINDING_CLEAR( lc ); ldap_pvt_thread_mutex_unlock( &li->li_conninfo.lai_mutex ); } @@ -650,6 +663,12 @@ ldap_back_dobind_int( return rc; } + while ( lc->lc_refcnt > 1 ) { + ldap_pvt_thread_yield(); + if (( rc = LDAP_BACK_CONN_ISBOUND( lc ))) + return rc; + } + /* * FIXME: we need to let clients use proxyAuthz * otherwise we cannot do symmetric pools of servers; @@ -1386,8 +1405,9 @@ ldap_back_proxy_authz_ctrl( /* just count ctrls */ ; } - ctrls = ch_malloc( sizeof( LDAPControl * ) * (i + 2) ); - ctrls[ 0 ] = ch_malloc( sizeof( LDAPControl ) ); + ctrls = op->o_tmpalloc( sizeof( LDAPControl * ) * (i + 2) + sizeof( LDAPControl ), + op->o_tmpmemctx ); + ctrls[ 0 ] = (LDAPControl *)&ctrls[ i + 2 ]; ctrls[ 0 ]->ldctl_oid = LDAP_CONTROL_PROXY_AUTHZ; ctrls[ 0 ]->ldctl_iscritical = 1; @@ -1396,13 +1416,14 @@ ldap_back_proxy_authz_ctrl( /* already in u:ID or dn:DN form */ case LDAP_BACK_IDASSERT_OTHERID: case LDAP_BACK_IDASSERT_OTHERDN: - ber_dupbv( &ctrls[ 0 ]->ldctl_value, &assertedID ); + ber_dupbv_x( &ctrls[ 0 ]->ldctl_value, &assertedID, op->o_tmpmemctx ); break; /* needs the dn: prefix */ default: ctrls[ 0 ]->ldctl_value.bv_len = assertedID.bv_len + STRLENOF( "dn:" ); - ctrls[ 0 ]->ldctl_value.bv_val = ch_malloc( ctrls[ 0 ]->ldctl_value.bv_len + 1 ); + ctrls[ 0 ]->ldctl_value.bv_val = op->o_tmpalloc( ctrls[ 0 ]->ldctl_value.bv_len + 1, + op->o_tmpmemctx ); AC_MEMCPY( ctrls[ 0 ]->ldctl_value.bv_val, "dn:", STRLENOF( "dn:" ) ); AC_MEMCPY( &ctrls[ 0 ]->ldctl_value.bv_val[ STRLENOF( "dn:" ) ], assertedID.bv_val, assertedID.bv_len + 1 ); @@ -1438,11 +1459,10 @@ ldap_back_proxy_authz_ctrl_free( Operation *op, LDAPControl ***pctrls ) assert( ctrls[ 0 ] != NULL ); if ( !BER_BVISNULL( &ctrls[ 0 ]->ldctl_value ) ) { - free( ctrls[ 0 ]->ldctl_value.bv_val ); + op->o_tmpfree( ctrls[ 0 ]->ldctl_value.bv_val, op->o_tmpmemctx ); } - free( ctrls[ 0 ] ); - free( ctrls ); + op->o_tmpfree( ctrls, op->o_tmpmemctx ); } *pctrls = NULL; diff --git a/servers/slapd/back-meta/bind.c b/servers/slapd/back-meta/bind.c index 54c669e7fc..32389a75c1 100644 --- a/servers/slapd/back-meta/bind.c +++ b/servers/slapd/back-meta/bind.c @@ -91,7 +91,7 @@ meta_back_bind( Operation *op, SlapReply *rs ) /* we need meta_back_getconn() not send result even on error, * because we want to intercept the error and make it * invalidCredentials */ - mc = meta_back_getconn( op, rs, NULL, LDAP_BACK_DONTSEND ); + mc = meta_back_getconn( op, rs, NULL, LDAP_BACK_BIND_DONTSEND ); if ( !mc ) { char buf[ SLAP_TEXT_BUFLEN ]; diff --git a/servers/slapd/back-meta/conn.c b/servers/slapd/back-meta/conn.c index b92b285f28..08fcbea92c 100644 --- a/servers/slapd/back-meta/conn.c +++ b/servers/slapd/back-meta/conn.c @@ -755,21 +755,31 @@ meta_back_getconn( } } - /* Searches for a metaconn in the avl tree */ - ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex ); - mc = (metaconn_t *)avl_find( mi->mi_conninfo.lai_tree, - (caddr_t)&mc_curr, meta_back_conn_cmp ); - if ( mc ) { - if ( mc->mc_tainted ) { - rs->sr_err = LDAP_UNAVAILABLE; - rs->sr_text = "remote server unavailable"; - ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex ); - return NULL; - } + /* Explicit Bind requests always get their own conn */ + if ( !( sendok & LDAP_BACK_BINDING ) ) { + /* Searches for a metaconn in the avl tree */ +retry_lock: + ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex ); + mc = (metaconn_t *)avl_find( mi->mi_conninfo.lai_tree, + (caddr_t)&mc_curr, meta_back_conn_cmp ); + if ( mc ) { + if ( mc->mc_tainted ) { + rs->sr_err = LDAP_UNAVAILABLE; + rs->sr_text = "remote server unavailable"; + ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex ); + return NULL; + } - mc->mc_refcnt++; + /* Don't reuse connections while they're still binding */ + if ( LDAP_BACK_CONN_BINDING( mc ) ) { + ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex ); + ldap_pvt_thread_yield(); + goto retry_lock; + } + mc->mc_refcnt++; + } + ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex ); } - ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex ); switch ( op->o_tag ) { case LDAP_REQ_ADD: @@ -822,6 +832,9 @@ meta_back_getconn( mc->mc_conn = mc_curr.mc_conn; ber_dupbv( &mc->mc_local_ndn, &mc_curr.mc_local_ndn ); new_conn = 1; + if ( sendok & LDAP_BACK_BINDING ) { + LDAP_BACK_CONN_BINDING_SET( mc ); + } } for ( i = 0; i < mi->mi_ntargets; i++ ) { @@ -947,13 +960,15 @@ meta_back_getconn( /* Retries searching for a metaconn in the avl tree * the reason is that the connection might have been * created by meta_back_get_candidate() */ - ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex ); - mc = (metaconn_t *)avl_find( mi->mi_conninfo.lai_tree, - (caddr_t)&mc_curr, meta_back_conn_cmp ); - if ( mc != NULL ) { - mc->mc_refcnt++; + if ( !( sendok & LDAP_BACK_BINDING ) ) { + ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex ); + mc = (metaconn_t *)avl_find( mi->mi_conninfo.lai_tree, + (caddr_t)&mc_curr, meta_back_conn_cmp ); + if ( mc != NULL ) { + mc->mc_refcnt++; + } + ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex ); } - ldap_pvt_thread_mutex_unlock( &mi->mi_conninfo.lai_mutex ); /* Looks like we didn't get a bind. Open a new session... */ if ( mc == NULL ) { @@ -961,6 +976,9 @@ meta_back_getconn( mc->mc_conn = mc_curr.mc_conn; ber_dupbv( &mc->mc_local_ndn, &mc_curr.mc_local_ndn ); new_conn = 1; + if ( sendok & LDAP_BACK_BINDING ) { + LDAP_BACK_CONN_BINDING_SET( mc ); + } } } @@ -1015,6 +1033,9 @@ meta_back_getconn( mc->mc_conn = mc_curr.mc_conn; ber_dupbv( &mc->mc_local_ndn, &mc_curr.mc_local_ndn ); new_conn = 1; + if ( sendok & LDAP_BACK_BINDING ) { + LDAP_BACK_CONN_BINDING_SET( mc ); + } } for ( i = 0; i < mi->mi_ntargets; i++ ) { @@ -1165,6 +1186,7 @@ meta_back_release_conn( ldap_pvt_thread_mutex_lock( &mi->mi_conninfo.lai_mutex ); assert( mc->mc_refcnt > 0 ); mc->mc_refcnt--; + LDAP_BACK_CONN_BINDING_CLEAR( mc ); if ( mc->mc_refcnt == 0 && mc->mc_tainted ) { (void)avl_delete( &mi->mi_conninfo.lai_tree, ( caddr_t )mc, meta_back_conn_cmp ); diff --git a/servers/slapd/back-monitor/thread.c b/servers/slapd/back-monitor/thread.c index 465e864519..2e4ded4535 100644 --- a/servers/slapd/back-monitor/thread.c +++ b/servers/slapd/back-monitor/thread.c @@ -271,9 +271,9 @@ monitor_subsys_thread_update( break; case 2: - for ( i = 0; !BER_BVISNULL( a->a_vals + i ); i++) { + for ( i = 0; !BER_BVISNULL( &a->a_vals[ i ] ); i++ ) { ch_free( a->a_vals[i].bv_val ); - BER_BVZERO( a->a_vals + i ); + BER_BVZERO( &a->a_vals[ i ] ); } bv.bv_val = buf; ldap_pvt_thread_mutex_lock( &slapd_rq.rq_mutex ); @@ -283,6 +283,12 @@ monitor_subsys_thread_update( value_add_one( &a->a_vals, &bv ); } ldap_pvt_thread_mutex_unlock( &slapd_rq.rq_mutex ); + + /* don't leave 'round attributes with no values */ + if ( BER_BVISNULL( &a->a_vals[ 0 ] ) ) { + BER_BVSTR( &bv, "()" ); + value_add_one( &a->a_vals, &bv ); + } break; } -- 2.39.5