Issue #23458: On POSIX, the file descriptor kept open by os.urandom() is now

vstinner · vstinner · commit e0a0bd6eaa40 · 2015-02-24T14:30:43.000+01:00
set to non inheritable
diff --git a/Lib/test/subprocessdata/fd_status.py b/Lib/test/subprocessdata/fd_status.py
@@ -0,0 +1,32 @@
+"""When called as a script, print a comma-separated list of the open
+file descriptors on stdout.
+
+Usage:
+fd_stats.py: check all file descriptors
+fd_status.py fd1 fd2 ...: check only specified file descriptors
+"""
+
+import errno
+import os
+import stat
+import sys
+
+if __name__ == "__main__":
+    fds = []
+    if len(sys.argv) == 1:
+        try:
+            _MAXFD = os.sysconf("SC_OPEN_MAX")
+        except:
+            _MAXFD = 256
+        test_fds = range(0, _MAXFD)
+    else:
+        test_fds = map(int, sys.argv[1:])
+    for fd in test_fds:
+        try:
+            st = os.fstat(fd)
+        except OSError as e:
+            if e.errno == errno.EBADF:
+                continue
+            raise
+        fds.append(fd)
+    print(','.join(map(str, fds)))
diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py
@@ -10,6 +10,7 @@
 import signal
 import subprocess
 import sysconfig
+import textwrap
 import time
 try:
     import resource
@@ -568,6 +569,33 @@ def test_urandom_subprocess(self):
         data2 = self.get_urandom_subprocess(16)
         self.assertNotEqual(data1, data2)
 
+    def test_urandom_fd_non_inheritable(self):
+        # Issue #23458: os.urandom() keeps a file descriptor open, but it
+        # must be non inheritable
+        fd_status = test_support.findfile("fd_status.py", subdir="subprocessdata")
+
+        # Need a two subprocesses because the Python test suite opens other
+        # inheritable file descriptors, whereas the test is specific to
+        # os.urandom() file descriptor
+        code = textwrap.dedent("""
+            import os
+            import subprocess
+            import sys
+
+            # Ensure that the /dev/urandom file descriptor is open
+            os.urandom(1)
+
+            exitcode = subprocess.call([sys.executable, %r],
+                                       close_fds=False)
+            sys.exit(exitcode)
+        """ % fd_status)
+
+        proc = subprocess.Popen([sys.executable, "-c", code],
+                                stdout=subprocess.PIPE, close_fds=True)
+        output, error = proc.communicate()
+        open_fds = set(map(int, output.rstrip().split(',')))
+        self.assertEqual(open_fds - set(range(3)), set())
+
 
 HAVE_GETENTROPY = (sysconfig.get_config_var('HAVE_GETENTROPY') == 1)
 
diff --git a/Misc/NEWS b/Misc/NEWS
@@ -18,6 +18,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #23458: On POSIX, the file descriptor kept open by os.urandom() is now
+  set to non inheritable
+
 - Issue #22113: struct.pack_into() now supports new buffer protocol (in
   particular accepts writable memoryview).
 
diff --git a/Python/random.c b/Python/random.c
@@ -188,6 +188,7 @@ dev_urandom_python(char *buffer, Py_ssize_t size)
     int fd;
     Py_ssize_t n;
     struct stat st;
+    int attr;
 
     if (size <= 0)
         return 0;
@@ -219,6 +220,14 @@ dev_urandom_python(char *buffer, Py_ssize_t size)
                 PyErr_SetFromErrno(PyExc_OSError);
             return -1;
         }
+
+        /* try to make the file descriptor non-inheritable, ignore errors */
+        attr = fcntl(fd, F_GETFD);
+        if (attr >= 0) {
+            attr |= FD_CLOEXEC;
+            (void)fcntl(fd, F_SETFD, attr);
+        }
+
         if (urandom_cache.fd >= 0) {
             /* urandom_fd was initialized by another thread while we were
                not holding the GIL, keep it. */
