(ITS#8450) race condition in ldap_int_utils_init

[doug.leavitt@oracle.com]

Full_Name: Doug Leavitt
Version: 2.4.44
OS: Solaris
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (137.254.4.13)


There is a race condition in ldap_int_utils_init that can be triggered when
multiple threads enter ldap_int_utils_init from ldap_init_initialize about the
same time. The done flag gets set immediately, before the various mutexes are
initialized. If thread A sets done, and thread B tests for done==1 before thread
A has completed the mutex inits, thread B can attempt to use an uninitialized
mutex and fail/core dump etc.

Additionally if judt the done=1 is moved to the bottom of the function thwo
threads can both be initializing the same mutexes multiple times causes other
mayhem.

The short term workaround for Solaris (THR APIs) is to move setting of done=1 to
after the mutex inits, and to protect the mutex inits using another statically
initialized mutex within ldap_int_utils_init.

Example patch:

-- util-int.c.~1~	2016-02-05 17:57:45.000000000 -0600
+++ util-int.c	2016-06-24 13:20:09.793308570 -0500
@@ -89,12 +89,16 @@
 # endif
 # if defined(HAVE_GETHOSTBYADDR_R) && \
 	(GETHOSTBDDDDR_R_NARGS < 7) || (8 < GETHOSTBYADDR_R_NARGS)
 	/* Don't know how to handle this version, pretend it's not there */
 #	undef HAVE_GETHOSTBYADDR_R
 # endif
+#if defined( HAVE_THR )
+	/* use static mutex to protect mutex initialization */
+static mutex_t ldap_int_utils_init_mutex = DEFAULTMUTEX;
+#endif /* HAVE_THR */
 #endif /* LDAP_R_COMPILE */
 
 char *ldap_pvt_ctime( const time_t *tp, char *buf )
 {
 #ifdef USE_CTIME_R
 # if (CTIME_R_NARGS > 3) || (CTIME_R_NARGS < 2)
@@69697,15 +701,22 @@
 
 void ldap_int_utils_init( void )
 {
 	static int done=0;
 	if (done)
 	  return;
-	done=1;
 
 #ifdef LDAP_R_COMPILE
+#if defined( HAVE_THR )
+	/* use static mutex to protect mutex initialization */
+	(void) mutex_lock(&ldap_int_utils_init_mutex);
+	if (done) {
+		(void) mutex_unlock(&ldap_int_utils_init_mutex);
+		return;
+	}
+#endif /* HAVE_THR */
 #if !defined( USE_CTIME_R ) && !defined( HAVE_REENTRANT_FUNCTIONS )
 	ldap_pvt_thread_mutex_init( &ldap_int_ctime_mutex );
 #endif
 #if !defined( USE_GMTIME_R ) && !defined( USE_LOCALTIME_R )
 	ldap_pvt_thread_mutex_init( &ldap_int_gmtime_mutex );
 #endif
@@ -715,15 +726,20 @@
 
 	ldap_pvt_thread_mutex_init( &ldap_int_gettime_mutex );
 
 #ifdef HAVE_GSSAPI
 	ldap_pvt_thread_mutex_init( &ldap_int_gssapi_mutex );
 #endif
-#endif
 
 	/* call other module init functions here... */
+	done=1;
+#if defined( HAVE_THR )
+	/* use static mutex to protect mutex initialization */
+	(void) mutex_unlock(&ldap_int_utils_init_mutex);
+#endif /* HAVE_THR */
+#endif
 }
 
 #if defined( NEED_COPY_HOSTENT )
 # undef NEED_SAFE_REALLOC
 #define NEED_SAFE_REALLOC
 

I know a similar workaround could be made for POSIX threads and possibly some of
the other supported thread models, but this approach seems kludgely.

I suspect the correct solution may be to somehow refactor ldap_int_utils_init
out of libldap and into libldap_r in a cleaner multi-platform manner than the
example above.