From 9bd90a741cb113b0986796b87e63c9e90e6432a7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Kuzn=C3=ADk?= <okuznik@symas.com>
Date: Thu, 8 Feb 2018 23:44:31 +0000
Subject: [PATCH] Fix a race on bind response processing.

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      | 34 ++++++++++++++++++++++++----------
 servers/lloadd/operation.c |  9 +++++++++
 servers/lloadd/upstream.c  |  7 -------
 3 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/servers/lloadd/bind.c b/servers/lloadd/bind.c
index 75820bdf19..51636727a2 100644
--- a/servers/lloadd/bind.c
+++ b/servers/lloadd/bind.c
@@ -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);
 
diff --git a/servers/lloadd/operation.c b/servers/lloadd/operation.c
index 06c75be4f5..54b8667afe 100644
--- a/servers/lloadd/operation.c
+++ b/servers/lloadd/operation.c
@@ -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 );
 
diff --git a/servers/lloadd/upstream.c b/servers/lloadd/upstream.c
index 41eaf77986..c490836380 100644
--- a/servers/lloadd/upstream.c
+++ b/servers/lloadd/upstream.c
@@ -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 {