diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -1070,6 +1070,49 @@ class URandomTests(unittest.TestCase): """ 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/Python/random.c b/Python/random.c --- a/Python/random.c +++ b/Python/random.c @@ -3,6 +3,9 @@ #include #else #include +#ifdef HAVE_SYS_STAT_H +#include +#endif #endif #ifdef Py_DEBUG @@ -69,7 +72,11 @@ win32_urandom(unsigned char *buffer, Py_ #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_ssiz { 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_ssiz 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 - urandom_fd = 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; + } + } } Py_BEGIN_ALLOW_THREADS @@ -168,9 +197,9 @@ dev_urandom_python(char *buffer, Py_ssiz 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; } }