md_rand.c thread safety

CHANGES

@@ -8,8 +8,8 @@
      and OpenSSL 0.9.7 were developped in parallel, based on OpenSSL 0.9.6.  
 
      Change log entries are tagged as follows:
-         -) applies to 0.9.6a/0.9.6b only
-         *) applies to 0.9.6a/0.9.6b and 0.9.7
+         -) applies to 0.9.6a/0.9.6b/0.9.6c only
+         *) applies to 0.9.6a/0.9.6b/0.9.6c and 0.9.7
          +) applies to 0.9.7 only
 
   *) In
@@ -24,6 +24,12 @@
      always reject numbers >= n.
      [Bodo Moeller]
 
+  *) In crypto/rand/md_rand.c, use a new short-time lock CRYPTO_LOCK_RAND2
+     to synchronize access to 'locking_thread'.  This is necessary on
+     systems where access to 'locking_thread' (an 'unsigned long'
+     variable) is not atomic.
+     [Bodo Moeller]
+
   *) In crypto/rand/md_rand.c, set 'locking_thread' to current thread's ID
      *before* setting the 'crypto_lock_rand' flag.  The previous code had
      a race condition if 0 is a valid thread ID.

crypto/cryptlib.c

@@ -90,6 +90,7 @@ static const char* lock_names[CRYPTO_NUM_LOCKS] =
 	"ssl_sess_cert",
 	"ssl",
 	"rand",
+	"rand2",
 	"debug_malloc",
 	"BIO",
 	"gethostbyname",
@@ -102,7 +103,7 @@ static const char* lock_names[CRYPTO_NUM_LOCKS] =
 	"dynlock",
 	"engine",
 	"ui",
-#if CRYPTO_NUM_LOCKS != 30
+#if CRYPTO_NUM_LOCKS != 31
 # error "Inconsistency between crypto.h and cryptlib.c"
 #endif
 	};

crypto/crypto.h

@@ -113,19 +113,20 @@ extern "C" {
 #define	CRYPTO_LOCK_SSL_SESS_CERT	15
 #define	CRYPTO_LOCK_SSL			16
 #define	CRYPTO_LOCK_RAND		17
-#define	CRYPTO_LOCK_MALLOC		18
-#define	CRYPTO_LOCK_BIO			19
-#define	CRYPTO_LOCK_GETHOSTBYNAME	20
-#define	CRYPTO_LOCK_GETSERVBYNAME	21
-#define	CRYPTO_LOCK_READDIR		22
-#define	CRYPTO_LOCK_RSA_BLINDING	23
-#define	CRYPTO_LOCK_DH			24
-#define	CRYPTO_LOCK_MALLOC2		25
-#define	CRYPTO_LOCK_DSO			26
-#define	CRYPTO_LOCK_DYNLOCK		27
-#define	CRYPTO_LOCK_ENGINE		28
-#define	CRYPTO_LOCK_UI			29
-#define	CRYPTO_NUM_LOCKS		30
+#define	CRYPTO_LOCK_RAND2		18
+#define	CRYPTO_LOCK_MALLOC		19
+#define	CRYPTO_LOCK_BIO			20
+#define	CRYPTO_LOCK_GETHOSTBYNAME	21
+#define	CRYPTO_LOCK_GETSERVBYNAME	22
+#define	CRYPTO_LOCK_READDIR		23
+#define	CRYPTO_LOCK_RSA_BLINDING	24
+#define	CRYPTO_LOCK_DH			25
+#define	CRYPTO_LOCK_MALLOC2		26
+#define	CRYPTO_LOCK_DSO			27
+#define	CRYPTO_LOCK_DYNLOCK		28
+#define	CRYPTO_LOCK_ENGINE		29
+#define	CRYPTO_LOCK_UI			30
+#define	CRYPTO_NUM_LOCKS		31
 
 #define CRYPTO_LOCK		1
 #define CRYPTO_UNLOCK		2

crypto/rand/md_rand.c

@@ -144,6 +144,7 @@ static int initialized=0;
 static unsigned int crypto_lock_rand = 0; /* may be set only when a thread
                                            * holds CRYPTO_LOCK_RAND
                                            * (to prevent double locking) */
+/* access to lockin_thread is synchronized by CRYPTO_LOCK_RAND2 */
 static unsigned long locking_thread = 0; /* valid iff crypto_lock_rand is set */
 
 
@@ -210,7 +211,14 @@ static void ssleay_rand_add(const void *buf, int num, double add)
 	 */
 
 	/* check if we already have the lock */
-	do_not_lock = crypto_lock_rand && (locking_thread == CRYPTO_thread_id());
+	if (crypto_lock_rand)
+		{
+		CRYPTO_r_lock(CRYPTO_LOCK_RAND2);
+		do_not_lock = (locking_thread == CRYPTO_thread_id());
+		CRYPTO_r_unlock(CRYPTO_LOCK_RAND2);
+		}
+	else
+		do_not_lock = 0;
 
 	if (!do_not_lock) CRYPTO_w_lock(CRYPTO_LOCK_RAND);
 	st_idx=state_index;
@@ -361,7 +369,9 @@ static int ssleay_rand_bytes(unsigned char *buf, int num)
 	CRYPTO_w_lock(CRYPTO_LOCK_RAND);
 
 	/* prevent ssleay_rand_bytes() from trying to obtain the lock again */
+	CRYPTO_w_lock(CRYPTO_LOCK_RAND2);
 	locking_thread = CRYPTO_thread_id();
+	CRYPTO_w_unlock(CRYPTO_LOCK_RAND2);
 	crypto_lock_rand = 1;
 
 	if (!initialized)
@@ -520,14 +530,23 @@ static int ssleay_rand_status(void)
 
 	/* check if we already have the lock
 	 * (could happen if a RAND_poll() implementation calls RAND_status()) */
-	do_not_lock = crypto_lock_rand && (locking_thread == CRYPTO_thread_id());
+	if (crypto_lock_rand)
+		{
+		CRYPTO_r_lock(CRYPTO_LOCK_RAND2);
+		do_not_lock = (locking_thread == CRYPTO_thread_id());
+		CRYPTO_r_unlock(CRYPTO_LOCK_RAND2);
+		}
+	else
+		do_not_lock = 0;
 	
 	if (!do_not_lock)
 		{
 		CRYPTO_w_lock(CRYPTO_LOCK_RAND);
 		
 		/* prevent ssleay_rand_bytes() from trying to obtain the lock again */
+		CRYPTO_w_lock(CRYPTO_LOCK_RAND2);
 		locking_thread = CRYPTO_thread_id();
+		CRYPTO_w_unlock(CRYPTO_LOCK_RAND2);
 		crypto_lock_rand = 1;
 		}