Fix races in sasl_{client,server}_init()

lib/client.c

@@ -279,6 +279,7 @@ client_idle(sasl_conn_t *conn)
 int sasl_client_init(const sasl_callback_t *callbacks)
 {
   int ret;
+  void *mutex = NULL;
   const add_plugin_list_t ep_list[] = {
       { "sasl_client_plug_init", (add_plugin_t *)sasl_client_add_plugin },
       { "sasl_canonuser_init", (add_plugin_t *)sasl_canonuser_add_plugin },
@@ -287,22 +288,30 @@ int sasl_client_init(const sasl_callback_t *callbacks)
 
   /* lock allocation type */
   _sasl_allocation_locked++;
-
-  if(_sasl_client_active) {
+
+  global_callbacks_client.callbacks = callbacks;
+  global_callbacks_client.appname = NULL;
+
+  ret = _sasl_common_init(&global_callbacks_client, &mutex);
+  if (ret != SASL_OK) return ret;
+
+  if (_sasl_client_active) {
       /* We're already active, just increase our refcount */
       /* xxx do something with the callback structure? */
       _sasl_client_active++;
+      sasl_MUTEX_UNLOCK(mutex);
       return SASL_OK;
   }
 
-  global_callbacks_client.callbacks = callbacks;
-  global_callbacks_client.appname = NULL;
-
   cmechlist=sasl_ALLOC(sizeof(cmech_list_t));
-  if (cmechlist==NULL) return SASL_NOMEM;
+  if (cmechlist==NULL) {
+      sasl_MUTEX_UNLOCK(mutex);
+      return SASL_NOMEM;
+  }
 
   /* We need to call client_done if we fail now */
   _sasl_client_active = 1;
+  sasl_MUTEX_UNLOCK(mutex);
 
   /* load plugins */
   ret=init_mechlist();  
@@ -313,8 +322,6 @@ int sasl_client_init(const sasl_callback_t *callbacks)
 
   sasl_client_add_plugin("EXTERNAL", &external_client_plug_init);
 
-  ret = _sasl_common_init(&global_callbacks_client);
-
   if (ret == SASL_OK)
       ret = _sasl_load_plugins(ep_list,
 			       _sasl_find_getpath_callback(callbacks),

lib/common.c

@@ -848,7 +848,7 @@ int _sasl_conn_init(sasl_conn_t *conn,
   RETURN(conn, SASL_OK);
 }
 
-int _sasl_common_init(sasl_global_callbacks_t *global_callbacks)
+int _sasl_common_init(sasl_global_callbacks_t *global_callbacks, void **mutex)
 {
     int result;
 
@@ -868,7 +868,7 @@ int _sasl_common_init(sasl_global_callbacks_t *global_callbacks)
 	global_utils->getopt = &_sasl_global_getopt;
 	global_utils->getopt_context = global_callbacks;
 
-	sasl_MUTEX_UNLOCK(free_mutex);
+	*mutex = free_mutex;
 	return SASL_OK;
     } else {
 	sasl_global_utils = _sasl_alloc_utils(NULL, global_callbacks);
@@ -884,7 +884,11 @@ int _sasl_common_init(sasl_global_callbacks_t *global_callbacks)
 	result = SASL_FAIL;
     }
 
-    sasl_MUTEX_UNLOCK(free_mutex);
+    if (result == SASL_OK) {
+	*mutex = free_mutex;
+    } else {
+	sasl_MUTEX_UNLOCK(free_mutex);
+    }
     return result;
 }
 

lib/saslint.h

@@ -384,7 +384,8 @@ _sasl_find_getconfpath_callback(const sasl_callback_t *callbacks);
 extern const sasl_callback_t *
 _sasl_find_verifyfile_callback(const sasl_callback_t *callbacks);
 
-extern int _sasl_common_init(sasl_global_callbacks_t *global_callbacks);
+extern int _sasl_common_init(sasl_global_callbacks_t *global_callbacks,
+			     void **mutex);
 
 extern int _sasl_conn_init(sasl_conn_t *conn,
 			   const char *service,

lib/server.c

@@ -826,6 +826,7 @@ int sasl_server_init(const sasl_callback_t *callbacks,
     int ret;
     const sasl_callback_t *vf;
     const char *pluginfile = NULL;
+    void *mutex = NULL;
 #ifdef PIC
     sasl_getopt_t *getopt;
     void *context;
@@ -845,32 +846,33 @@ int sasl_server_init(const sasl_callback_t *callbacks,
     if (appname != NULL && strlen(appname) >= PATH_MAX)
 	return SASL_BADPARAM;
 
-    if (_sasl_server_active) {
-	/* We're already active, just increase our refcount */
-	/* xxx do something with the callback structure? */
-	_sasl_server_active++;
-	return SASL_OK;
-    }
-
-    ret = _sasl_common_init(&global_callbacks);
-    if (ret != SASL_OK)
-	return ret;
-
     /* verify that the callbacks look ok */
     ret = verify_server_callbacks(callbacks);
-    if (ret != SASL_OK)
+    if (ret != SASL_OK) {
+	sasl_MUTEX_UNLOCK(mutex);
 	return ret;
+    }
 
     global_callbacks.callbacks = callbacks;
-    
+
     /* A shared library calling sasl_server_init will pass NULL as appname.
        This should retain the original appname. */
     if (appname != NULL) {
         global_callbacks.appname = appname;
     }
 
+    ret = _sasl_common_init(&global_callbacks, &mutex);
+    if (ret != SASL_OK) return ret;
+
+    if (_sasl_server_active++) {
+	/* We're already active */
+	/* xxx do something with the callback structure? */
+	sasl_MUTEX_UNLOCK(mutex);
+	return SASL_OK;
+    }
+
     /* If we fail now, we have to call server_done */
-    _sasl_server_active = 1;
+    sasl_MUTEX_UNLOCK(mutex);
 
     /* allocate mechlist and set it to empty */
     mechlist = sasl_ALLOC(sizeof(mech_list_t));