Fixed slapd-ldap/meta session reuse issue (ITS#3415)

Fixed slapd-monitor thread issue (ITS#3418)

CHANGES

 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)

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

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 */

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;

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 ];
 

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 );

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;
 	}