Issue python#21207: Detect when the os.urandom cached fd has been closed or replaced, and open it anew.

pitrou · pitrou · commit e472aeafc3ce · 2014-04-26T14:33:03.000+02:00
diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py
@@ -1070,6 +1070,49 @@ def test_urandom_failure(self):
             """
         assert_python_ok('-c', code)
 
+    def test_urandom_fd_closed(self):
+        # Issue #21207: urandom() should reopen its fd to /dev/urandom if
+        # closed.
+        code = """if 1:
+            import os
+            import sys
+            os.urandom(4)
+            os.closerange(3, 256)
+            sys.stdout.buffer.write(os.urandom(4))
+            """
+        rc, out, err = assert_python_ok('-Sc', code)
+
+    def test_urandom_fd_reopened(self):
+        # Issue #21207: urandom() should detect its fd to /dev/urandom
+        # changed to something else, and reopen it.
+        with open(support.TESTFN, 'wb') as f:
+            f.write(b"x" * 256)
+        self.addCleanup(os.unlink, support.TESTFN)
+        code = """if 1:
+            import os
+            import sys
+            os.urandom(4)
+            for fd in range(3, 256):
+                try:
+                    os.close(fd)
+                except OSError:
+                    pass
+                else:
+                    # Found the urandom fd (XXX hopefully)
+                    break
+            os.closerange(3, 256)
+            with open({TESTFN!r}, 'rb') as f:
+                os.dup2(f.fileno(), fd)
+                sys.stdout.buffer.write(os.urandom(4))
+                sys.stdout.buffer.write(os.urandom(4))
+            """.format(TESTFN=support.TESTFN)
+        rc, out, err = assert_python_ok('-Sc', code)
+        self.assertEqual(len(out), 8)
+        self.assertNotEqual(out[0:4], out[4:8])
+        rc, out2, err2 = assert_python_ok('-Sc', code)
+        self.assertEqual(len(out2), 8)
+        self.assertNotEqual(out2, out)
+
 
 @contextlib.contextmanager
 def _execvpe_mockup(defpath=None):
diff --git a/Misc/NEWS b/Misc/NEWS
@@ -39,6 +39,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #21207: Detect when the os.urandom cached fd has been closed or
+  replaced, and open it anew.
+
 - Issue #21291: subprocess's Popen.wait() is now thread safe so that
   multiple threads may be calling wait() or poll() on a Popen instance
   at the same time without losing the Popen.returncode value.
diff --git a/Python/random.c b/Python/random.c
@@ -3,6 +3,9 @@
 #include <windows.h>
 #else
 #include <fcntl.h>
+#ifdef HAVE_SYS_STAT_H
+#include <sys/stat.h>
+#endif
 #endif
 
 #ifdef Py_DEBUG
@@ -69,7 +72,11 @@ win32_urandom(unsigned char *buffer, Py_ssize_t size, int raise)
 
 
 #ifndef MS_WINDOWS
-static int urandom_fd = -1;
+static struct {
+    int fd;
+    dev_t st_dev;
+    ino_t st_ino;
+} urandom_cache = { -1 };
 
 /* Read size bytes from /dev/urandom into buffer.
    Call Py_FatalError() on error. */
@@ -109,12 +116,24 @@ dev_urandom_python(char *buffer, Py_ssize_t size)
 {
     int fd;
     Py_ssize_t n;
+    struct stat st;
 
     if (size <= 0)
         return 0;
 
-    if (urandom_fd >= 0)
-        fd = urandom_fd;
+    if (urandom_cache.fd >= 0) {
+        /* Does the fd point to the same thing as before? (issue #21207) */
+        if (fstat(urandom_cache.fd, &st)
+            || st.st_dev != urandom_cache.st_dev
+            || st.st_ino != urandom_cache.st_ino) {
+            /* Something changed: forget the cached fd (but don't close it,
+               since it probably points to something important for some
+               third-party code). */
+            urandom_cache.fd = -1;
+        }
+    }
+    if (urandom_cache.fd >= 0)
+        fd = urandom_cache.fd;
     else {
         Py_BEGIN_ALLOW_THREADS
         fd = _Py_open("/dev/urandom", O_RDONLY);
@@ -129,14 +148,24 @@ dev_urandom_python(char *buffer, Py_ssize_t size)
                 PyErr_SetFromErrno(PyExc_OSError);
             return -1;
         }
-        if (urandom_fd >= 0) {
+        if (urandom_cache.fd >= 0) {
             /* urandom_fd was initialized by another thread while we were
                not holding the GIL, keep it. */
             close(fd);
-            fd = urandom_fd;
+            fd = urandom_cache.fd;
+        }
+        else {
+            if (fstat(fd, &st)) {
+                PyErr_SetFromErrno(PyExc_OSError);
+                close(fd);
+                return -1;
+            }
+            else {
+                urandom_cache.fd = fd;
+                urandom_cache.st_dev = st.st_dev;
+                urandom_cache.st_ino = st.st_ino;
+            }
         }
-        else
-            urandom_fd = fd;
     }
 
     Py_BEGIN_ALLOW_THREADS
@@ -168,9 +197,9 @@ dev_urandom_python(char *buffer, Py_ssize_t size)
 static void
 dev_urandom_close(void)
 {
-    if (urandom_fd >= 0) {
-        close(urandom_fd);
-        urandom_fd = -1;
+    if (urandom_cache.fd >= 0) {
+        close(urandom_cache.fd);
+        urandom_cache.fd = -1;
     }
 }
 
