]> git.ipfire.org Git - thirdparty/openldap.git/commitdiff
Fix a race on bind response processing.
authorOndřej Kuzník <okuznik@symas.com>
Thu, 8 Feb 2018 23:44:31 +0000 (23:44 +0000)
committerOndřej Kuzník <okuznik@symas.com>
Tue, 17 Nov 2020 17:58:14 +0000 (17:58 +0000)
During response processing, an upstream connection could be marked ready
after a different bind had already been allocated to it, thus allowing
two binds to be in progress on the same connection.

servers/lloadd/bind.c
servers/lloadd/operation.c
servers/lloadd/upstream.c

index 75820bdf19ce18449c93d93b7861dbe69892d965..51636727a214ca3ba07d112fbcea3fd0cc6ef765 100644 (file)
@@ -374,8 +374,9 @@ request_bind( LloadConnection *client, LloadOperation *op )
     if ( ber == NULL && (ber = ber_alloc()) == NULL ) {
         Debug( LDAP_DEBUG_ANY, "request_bind: "
                 "ber_alloc failed\n" );
-        ldap_pvt_thread_mutex_unlock( &upstream->c_io_mutex );
         CONNECTION_LOCK_DECREF(upstream);
+        ldap_pvt_thread_mutex_unlock( &upstream->c_io_mutex );
+        upstream->c_state = LLOAD_C_READY;
         if ( !BER_BVISNULL( &upstream->c_sasl_bind_mech ) ) {
             ber_memfree( upstream->c_sasl_bind_mech.bv_val );
             BER_BVZERO( &upstream->c_sasl_bind_mech );
@@ -605,7 +606,28 @@ handle_bind_response(
             op->o_client_msgid, op->o_client_connid, result );
 
     CONNECTION_LOCK(upstream);
-    if ( result != LDAP_SASL_BIND_IN_PROGRESS ) {
+    if ( !tavl_find( upstream->c_ops, op, operation_upstream_cmp ) ) {
+        /*
+         * operation might not be found because:
+         * - it has timed out (only happens when debugging/hung/...)
+         *   a response has been sent for us, we must not send another
+         * - it has been abandoned (new bind, unbind)
+         *   no response is expected
+         * - ???
+         */
+        operation_destroy_from_upstream( op );
+        CONNECTION_UNLOCK(upstream);
+        return LDAP_SUCCESS;
+    }
+
+    if ( result == LDAP_SASL_BIND_IN_PROGRESS ) {
+        tavl_delete( &upstream->c_ops, op, operation_upstream_cmp );
+        op->o_upstream_msgid = 0;
+        op->o_upstream_refcnt++;
+        rc = tavl_insert(
+                &upstream->c_ops, op, operation_upstream_cmp, avl_dup_error );
+        assert( rc == LDAP_SUCCESS );
+    } else {
         int sasl_finished = 0;
         if ( !BER_BVISNULL( &upstream->c_sasl_bind_mech ) ) {
             sasl_finished = 1;
@@ -620,14 +642,6 @@ handle_bind_response(
             return finish_sasl_bind( upstream, op, ber );
         }
         upstream->c_state = LLOAD_C_READY;
-    } else {
-        if ( tavl_delete( &upstream->c_ops, op, operation_upstream_cmp ) ) {
-            op->o_upstream_msgid = 0;
-            op->o_upstream_refcnt++;
-            rc = tavl_insert( &upstream->c_ops, op, operation_upstream_cmp,
-                    avl_dup_error );
-            assert( rc == LDAP_SUCCESS );
-        }
     }
     CONNECTION_UNLOCK(upstream);
 
index 06c75be4f58fcca2460d7a23ed1268b302b0e199..54b8667afecd4f21fdad6946c32b86ab8d9d0d42 100644 (file)
@@ -837,6 +837,15 @@ connection_timeout( LloadConnection *upstream, time_t threshold )
         found_op = tavl_delete( &upstream->c_ops, op, operation_upstream_cmp );
         assert( op == found_op );
 
+        if ( upstream->c_state == LLOAD_C_BINDING ) {
+            assert( op->o_tag == LDAP_REQ_BIND && upstream->c_ops == NULL );
+            upstream->c_state = LLOAD_C_READY;
+            if ( !BER_BVISNULL( &upstream->c_sasl_bind_mech ) ) {
+                ber_memfree( upstream->c_sasl_bind_mech.bv_val );
+                BER_BVZERO( &upstream->c_sasl_bind_mech );
+            }
+        }
+
         rc = tavl_insert( &ops, op, operation_upstream_cmp, avl_dup_error );
         assert( rc == LDAP_SUCCESS );
 
index 41eaf779862495ffc158594fba209bd7f6ea5e0f..c4908363800cbfd2337497436a14137437d62efa 100644 (file)
@@ -257,13 +257,6 @@ handle_one_response( LloadConnection *c )
         CONNECTION_LOCK_DECREF(c);
         op->o_upstream_refcnt--;
         if ( !client || !op->o_upstream_refcnt ) {
-            if ( c->c_state == LLOAD_C_BINDING ) {
-                c->c_state = LLOAD_C_READY;
-                if ( !BER_BVISNULL( &c->c_sasl_bind_mech ) ) {
-                    ber_memfree( c->c_sasl_bind_mech.bv_val );
-                    BER_BVZERO( &c->c_sasl_bind_mech );
-                }
-            }
             operation_destroy_from_upstream( op );
         }
     } else {