Work around for /dev/urandom race condition

1 files changed, 93 insertions, 35 deletions

diff --git a/cpp/src/IceUtil/UUID.cpp b/cpp/src/IceUtil/UUID.cpp
index 0654c81a1b6..4b6e62308a9 100644
--- a/cpp/src/IceUtil/UUID.cpp
+++ b/cpp/src/IceUtil/UUID.cpp
@@ -17,7 +17,8 @@
 #ifdef _WIN32
 #   include <rpc.h>
 #else
-#include <sys/types.h>
+#   include <IceUtil/StaticMutex.h>
+#   include <sys/types.h>
 #   include <sys/stat.h>
 #   include <fcntl.h>
 #   include <unistd.h>
@@ -56,6 +57,48 @@ IceUtil::UUIDGenerationException::UUIDGenerationException(const char* file, int
 
 string IceUtil::UUIDGenerationException::_name = "IceUtil::UUIDGenerationException";
 
+#ifndef _WIN32
+//
+// Unfortunately on Linux (at least up to 2.6.9), concurrent access to /dev/urandom
+// can return the same value. Search for "Concurrent access to /dev/urandom" in the 
+// linux-kernel mailing list archive for additional details.
+// Since /dev/urandom on other platforms is usually a port from Linux, this problem 
+// could be widespread. Therefore, instead of using 122 random bits that could be 
+// duplicated, we replace the last 15 bits of all "random" UUIDs by the last 15 bits 
+// of the process id, and in each process, we serialize access to /dev/urandom using 
+// a static mutex.
+//
+
+static IceUtil::StaticMutex staticMutex = ICE_STATIC_MUTEX_INITIALIZER;
+static int fd = -1;
+static char myPid[2];
+
+namespace
+{
+
+//
+// Close fd at exit
+//
+class UUIDCleanup
+{
+public:
+    
+    ~UUIDCleanup()
+    {
+         IceUtil::StaticMutex::Lock lock(staticMutex);
+	 if(fd != -1)
+	 {
+	     close(fd);
+	     fd = -1;
+	 }
+    }
+};
+static UUIDCleanup uuidCleanup;
+}
+
+
+#endif
+
 const string&
 IceUtil::UUIDGenerationException::ice_name() const
 {
@@ -93,20 +136,6 @@ IceUtil::generateUUID()
     return result;
     
 #else
-
-    // On Linux, /dev/random, even when used in blocking mode, sometimes 
-    // fails or returns fewer bytes.
-    // Maybe we should use a combination of version 4 UUIDs (with /dev/random),
-    // and version 1 UUIDs (MAC address + time), when /dev/random is exhausted?
-
-    int fd = open("/dev/urandom", O_RDONLY);
-
-    if (fd == -1)
-    {
-	assert(0);
-	throw UUIDGenerationException(__FILE__, __LINE__);
-    }
-
     struct UUID
     {
 	unsigned char timeLow[4];
@@ -124,37 +153,59 @@ IceUtil::generateUUID()
     int reads = 0;
     size_t index = 0;
 
-    // Limit the number of attempts to 20 reads to avoid
-    // a potential "for ever" loop
-    //
-    while(reads <= 20 && index != sizeof(UUID))
     {
-	ssize_t bytesRead = read(fd, buffer + index, sizeof(UUID) - index);
-	
-	if(bytesRead == -1 && errno != EINTR)
+	//
+	// Serialize access to /dev/urandom; see comment above.
+	//
+	IceUtil::StaticMutex::Lock lock(staticMutex);
+	if(fd == -1)
 	{
-	    int err = errno;
-	    cerr << "Reading /dev/urandom returned " << strerror(err) << endl;
-	    assert(0);
-	    close(fd);
-	    throw UUIDGenerationException(__FILE__, __LINE__);
+	    fd = open("/dev/urandom", O_RDONLY);
+	    if (fd == -1)
+	    {
+		assert(0);
+		throw UUIDGenerationException(__FILE__, __LINE__);
+	    }
+	    
+	    //
+	    // Initialize myPid as well
+	    // 
+	    pid_t pid = getpid();
+	    myPid[0] = (pid >> 8) & 0x7F;
+	    myPid[1] = pid & 0xFF;
 	}
-	else
+	
+
+	//
+	// Limit the number of attempts to 20 reads to avoid
+	// a potential "for ever" loop
+	//
+	while(reads <= 20 && index != sizeof(UUID))
 	{
-	    index += bytesRead;
-	    reads++;
+	    ssize_t bytesRead = read(fd, buffer + index, sizeof(UUID) - index);
+	    
+	    if(bytesRead == -1 && errno != EINTR)
+	    {
+		int err = errno;
+		cerr << "Reading /dev/urandom returned " << strerror(err) << endl;
+		assert(0);
+		throw UUIDGenerationException(__FILE__, __LINE__);
+	    }
+	    else
+	    {
+		index += bytesRead;
+		reads++;
+	    }
 	}
     }
-    
+	
     if (index != sizeof(UUID))
     {
 	assert(0);
-	close(fd);
 	throw UUIDGenerationException(__FILE__, __LINE__);
     }
-    
-    close(fd);
-    
+   
+    //
     // Adjust the bits that say "version 4" UUID
     //
     uuid.timeHighAndVersion[0] &= 0x0F;
@@ -162,6 +213,13 @@ IceUtil::generateUUID()
     uuid.clockSeqHiAndReserved &= 0x3F;
     uuid.clockSeqHiAndReserved |= 0x80;
 
+    //
+    // Replace the end of the node by myPid (15 bits) 
+    //
+    uuid.node[4] = (uuid.node[4] & 0x80) | myPid[0];
+    uuid.node[5] = myPid[1];
+
+    //
     // Convert to a UUID string
     //
     char uuidString[16 * 2 + 4 + 1]; // 16 bytes, 4 '-' and a final '\0'