From 11c0f99aaa2deead80bde7e35dd9f9aabac5cc20 Mon Sep 17 00:00:00 2001
From: Rich Megginson <rmeggins@redhat.com>
Date: May 28 2013 17:36:15 +0000
Subject: Ticket #47349 - DS instance crashes under a high load


https://fedorahosted.org/389/ticket/47349
Reviewed by: nkinder (Thanks!)
Branch: 389-ds-base-1.3.0
Fix Description: handle_new_connection initializes the connection object,
then calls connection_table_move_connection_on_to_active_list to put it
on the list of active connections, then unlocks the c_mutex, then calls
connection_new_private to allocate c_private.  If another thread
interrupts after the conn has been moved to the active list, but before
c_private has been allocated, the new conn will be available via
connection_table_iterate_active_connections where table_iterate_function
will attempt to dereference the NULL c_private.
The fix is to move connection_new_private inside the c_mutex lock, and to
move connection_table_move_connection_on_to_active_list to be the very last
thing before releasing the c_mutex lock.  Once the conn is on the active
list it is live and we cannot do anything else to it.
Note: I have still not been able to reproduce the problem in a non-debug
optimized build.
Platforms tested: RHEL6 x86_64
Note: Before patch, server would crash within 5 minutes.  After patch, server
has been running for several days in customer environment.
Flag Day: no
Doc impact: no
(cherry picked from commit 05d209432571dc64b242ae47113ae4cbb43607d2)

---

diff --git a/ldap/servers/slapd/daemon.c b/ldap/servers/slapd/daemon.c
index 1fd40f6..18ca091 100644
--- a/ldap/servers/slapd/daemon.c
+++ b/ldap/servers/slapd/daemon.c
@@ -2699,16 +2699,6 @@ handle_new_connection(Connection_Table *ct, int tcps, PRFileDesc *pr_acceptfd, i
 	/* Call the plugin extension constructors */
 	conn->c_extension = factory_create_extension(connection_type,conn,NULL /* Parent */);
 
-
-	/* Add this connection slot to the doubly linked list of active connections.  This
-	 * list is used to find the connections that should be used in the poll call. This 
-	 * connection will be added directly after slot 0 which serves as the head of the list */
-	if ( conn != NULL && conn->c_next == NULL && conn->c_prev == NULL )
-	{
-		/* Now give the new connection to the connection code */
-		connection_table_move_connection_on_to_active_list(the_connection_table,conn);
-	}
-
 #if defined(ENABLE_LDAPI)
 #if !defined( XP_WIN32 )
 	/* ldapi */
@@ -2721,10 +2711,21 @@ handle_new_connection(Connection_Table *ct, int tcps, PRFileDesc *pr_acceptfd, i
 #endif
 #endif /* ENABLE_LDAPI */
 
-	PR_Unlock( conn->c_mutex );
-
 	connection_new_private(conn);
 
+	/* Add this connection slot to the doubly linked list of active connections.  This
+	 * list is used to find the connections that should be used in the poll call. This
+	 * connection will be added directly after slot 0 which serves as the head of the list.
+	 * This must be done as the very last thing before we unlock the mutex, because once it
+	 * is added to the active list, it is live. */
+	if ( conn != NULL && conn->c_next == NULL && conn->c_prev == NULL )
+	{
+		/* Now give the new connection to the connection code */
+		connection_table_move_connection_on_to_active_list(the_connection_table,conn);
+	}
+
+	PR_Unlock( conn->c_mutex );
+
 	g_increment_current_conn_count();
 
 	return 0;