From e0a0bd6eaa40c132e1a270df6ae73c1d87628188 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 24 Feb 2015 14:30:43 +0100 Subject: [PATCH] Issue #23458: On POSIX, the file descriptor kept open by os.urandom() is now set to non inheritable --- Lib/test/subprocessdata/fd_status.py | 32 ++++++++++++++++++++++++++++ Lib/test/test_os.py | 28 ++++++++++++++++++++++++ Misc/NEWS | 3 +++ Python/random.c | 9 ++++++++ 4 files changed, 72 insertions(+) create mode 100644 Lib/test/subprocessdata/fd_status.py diff --git a/Lib/test/subprocessdata/fd_status.py b/Lib/test/subprocessdata/fd_status.py new file mode 100644 index 000000000000..7d5c0e8eff1b --- /dev/null +++ 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 index e84f4571dbfa..e60faae44537 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -10,6 +10,7 @@ import sys import signal import subprocess import sysconfig +import textwrap import time try: import resource @@ -568,6 +569,33 @@ class URandomTests (unittest.TestCase): 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 index b49d5dd5ec85..be6896151a51 100644 --- 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 index da49bbab5a46..ad8993d9533e 100644 --- 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. */ -- 2.47.3