From: Ondřej Kuzník <okuznik@symas.com>
Date: Thu, 23 Sep 2021 09:17:18 +0000 (+0100)
Subject: ITS#9600 Do not hold locks while calling into back-monitor
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6cc6ec2f72127c0c7e6d0afddbfe52e32a9a215c;p=thirdparty%2Fopenldap.git

ITS#9600 Do not hold locks while calling into back-monitor
---

diff --git a/servers/lloadd/client.c b/servers/lloadd/client.c
index 0b6515d5bf..5f6d59cae6 100644
--- a/servers/lloadd/client.c
+++ b/servers/lloadd/client.c
@@ -600,22 +600,29 @@ client_init(
     }
     c->c_write_event = event;
 
+    CONNECTION_LOCK(c);
+#ifdef BALANCER_MODULE
+    if ( lload_monitor_client_subsys ) {
+        acquire_ref( &c->c_refcnt );
+        CONNECTION_UNLOCK(c);
+        if ( lload_monitor_conn_entry_create(
+                    c, lload_monitor_client_subsys ) ) {
+            CONNECTION_LOCK(c);
+            RELEASE_REF( c, c_refcnt, c->c_destroy );
+            goto fail;
+        }
+        CONNECTION_LOCK(c);
+        RELEASE_REF( c, c_refcnt, c->c_destroy );
+    }
+#endif /* BALANCER_MODULE */
+
     c->c_destroy = client_destroy;
     c->c_unlink = client_unlink;
     c->c_pdu_cb = handle_one_request;
 
-    CONNECTION_LOCK(c);
     /* We only register the write event when we have data pending */
     event_add( c->c_read_event, c->c_read_timeout );
 
-#ifdef BALANCER_MODULE
-    if ( lload_monitor_client_subsys &&
-            lload_monitor_conn_entry_create(
-                    c, lload_monitor_client_subsys ) ) {
-        goto fail;
-    }
-#endif /* BALANCER_MODULE */
-
     checked_lock( &clients_mutex );
     LDAP_CIRCLEQ_INSERT_TAIL( &clients, c, c_next );
     checked_unlock( &clients_mutex );
@@ -623,6 +630,14 @@ client_init(
 
     return c;
 fail:
+    if ( !IS_ALIVE( c, c_live ) ) {
+        /*
+         * Released while we were unlocked, it's scheduled for destruction
+         * already
+         */
+        return NULL;
+    }
+
     if ( c->c_write_event ) {
         event_free( c->c_write_event );
         c->c_write_event = NULL;
diff --git a/servers/lloadd/connection.c b/servers/lloadd/connection.c
index 7398734153..fbb6492cef 100644
--- a/servers/lloadd/connection.c
+++ b/servers/lloadd/connection.c
@@ -522,7 +522,7 @@ lload_connection_close( LloadConnection *c, void *arg )
             "marking connection connid=%lu closing\n",
             c->c_connid );
 
-    /* We were approached from the connection list */
+    /* We were approached from the connection list or cn=monitor */
     assert( IS_ALIVE( c, c_refcnt ) );
 
     /* Need to acquire this first, even if we won't need it */
diff --git a/servers/lloadd/upstream.c b/servers/lloadd/upstream.c
index e0329003fc..ef794b5525 100644
--- a/servers/lloadd/upstream.c
+++ b/servers/lloadd/upstream.c
@@ -963,15 +963,26 @@ upstream_init( ber_socket_t s, LloadBackend *b )
     /* We only add the write event when we have data pending */
     c->c_write_event = event;
 
-    c->c_destroy = upstream_destroy;
-    c->c_unlink = upstream_unlink;
-
 #ifdef BALANCER_MODULE
-    if ( b->b_monitor && lload_monitor_conn_entry_create( c, b->b_monitor ) ) {
-        goto fail;
+    if ( b->b_monitor ) {
+        acquire_ref( &c->c_refcnt );
+        CONNECTION_UNLOCK(c);
+        checked_unlock( &b->b_mutex );
+        if ( lload_monitor_conn_entry_create( c, b->b_monitor ) ) {
+            RELEASE_REF( c, c_refcnt, c->c_destroy );
+            checked_lock( &b->b_mutex );
+            CONNECTION_LOCK(c);
+            goto fail;
+        }
+        checked_lock( &b->b_mutex );
+        CONNECTION_LOCK(c);
+        RELEASE_REF( c, c_refcnt, c->c_destroy );
     }
 #endif /* BALANCER_MODULE */
 
+    c->c_destroy = upstream_destroy;
+    c->c_unlink = upstream_unlink;
+
 #ifdef HAVE_TLS
     if ( c->c_is_tls == LLOAD_CLEARTEXT ) {
 #endif /* HAVE_TLS */
@@ -1015,6 +1026,14 @@ upstream_init( ber_socket_t s, LloadBackend *b )
     return c;
 
 fail:
+    if ( !IS_ALIVE( c, c_live ) ) {
+        /*
+         * Released while we were unlocked, it's scheduled for destruction
+         * already
+         */
+        return NULL;
+    }
+
     if ( c->c_write_event ) {
         event_del( c->c_write_event );
         event_free( c->c_write_event );