Fix race condition in create_stdio()

Issue #24891: Fix a race condition at Python startup if the file descriptor
of stdin (0), stdout (1) or stderr (2) is closed while Python is creating
sys.stdin, sys.stdout and sys.stderr objects. These attributes are now set
to None if the creation of the object failed, instead of raising an OSError
exception. Initial patch written by Marco Paolini.

diff --git a/Misc/ACKS b/Misc/ACKS
index 9af7c98f040e33db990d5318a09690ff0f3d0a7d..948e542ca73053b95dc06d1832f5d91298b8c805 100644 (file)
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -1037,6 +1037,7 @@ Jan Palus
 Yongzhi Pan
 Martin Panter
 Mathias Panzenböck
+Marco Paolini
 M. Papillon
 Peter Parente
 Alexandre Parenteau

diff --git a/Misc/NEWS b/Misc/NEWS
index 02040903041807c0098e94a446673c603598d6bb..66780edd2709afad717a1b1622e3c98ab27a92e5 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,12 @@ Release date: tba
 Core and Builtins
 -----------------
 
+- Issue #24891: Fix a race condition at Python startup if the file descriptor
+  of stdin (0), stdout (1) or stderr (2) is closed while Python is creating
+  sys.stdin, sys.stdout and sys.stderr objects. These attributes are now set
+  to None if the creation of the object failed, instead of raising an OSError
+  exception. Initial patch written by Marco Paolini.
+
 - Issue #21167: NAN operations are now handled correctly when python is
   compiled with ICC even if -fp-model strict is not specified.
 

diff --git a/Python/pythonrun.c b/Python/pythonrun.c
index 6d6e1794a32de0cde72bb0df862108ff79de2969..0967a47c57481a290bbd1003849f9c520bcdd628 100644 (file)
--- a/Python/pythonrun.c
+++ b/Python/pythonrun.c
@@ -1003,6 +1003,21 @@ initsite(void)
     }
 }
 
+/* Check if a file descriptor is valid or not.
+   Return 0 if the file descriptor is invalid, return non-zero otherwise. */
+static int
+is_valid_fd(int fd)
+{
+    int fd2;
+    if (fd < 0 || !_PyVerify_fd(fd))
+        return 0;
+    fd2 = dup(fd);
+    if (fd2 >= 0)
+        close(fd2);
+    return fd2 >= 0;
+}
+
+/* returns Py_None if the fd is not valid */
 static PyObject*
 create_stdio(PyObject* io,
     int fd, int write_mode, char* name,
@@ -1018,6 +1033,9 @@ create_stdio(PyObject* io,
     _Py_IDENTIFIER(TextIOWrapper);
     _Py_IDENTIFIER(mode);
 
+    if (!is_valid_fd(fd))
+        Py_RETURN_NONE;
+
     /* stdin is always opened in buffered mode, first because it shouldn't
        make a difference in common use cases, second because TextIOWrapper
        depends on the presence of a read1() method which only exists on
@@ -1099,20 +1117,15 @@ error:
     Py_XDECREF(stream);
     Py_XDECREF(text);
     Py_XDECREF(raw);
-    return NULL;
-}
 
-static int
-is_valid_fd(int fd)
-{
-    int dummy_fd;
-    if (fd < 0 || !_PyVerify_fd(fd))
-        return 0;
-    dummy_fd = dup(fd);
-    if (dummy_fd < 0)
-        return 0;
-    close(dummy_fd);
-    return 1;
+    if (PyErr_ExceptionMatches(PyExc_OSError) && !is_valid_fd(fd)) {
+        /* Issue #24891: the file descriptor was closed after the first
+           is_valid_fd() check was called. Ignore the OSError and set the
+           stream to None. */
+        PyErr_Clear();
+        Py_RETURN_NONE;
+    }
+    return NULL;
 }
 
 /* Initialize sys.stdin, stdout, stderr and builtins.open */
@@ -1188,30 +1201,18 @@ initstdio(void)
      * and fileno() may point to an invalid file descriptor. For example
      * GUI apps don't have valid standard streams by default.
      */
-    if (!is_valid_fd(fd)) {
-        std = Py_None;
-        Py_INCREF(std);
-    }
-    else {
-        std = create_stdio(iomod, fd, 0, "<stdin>", encoding, errors);
-        if (std == NULL)
-            goto error;
-    } /* if (fd < 0) */
+    std = create_stdio(iomod, fd, 0, "<stdin>", encoding, errors);
+    if (std == NULL)
+        goto error;
     PySys_SetObject("__stdin__", std);
     _PySys_SetObjectId(&PyId_stdin, std);
     Py_DECREF(std);
 
     /* Set sys.stdout */
     fd = fileno(stdout);
-    if (!is_valid_fd(fd)) {
-        std = Py_None;
-        Py_INCREF(std);
-    }
-    else {
-        std = create_stdio(iomod, fd, 1, "<stdout>", encoding, errors);
-        if (std == NULL)
-            goto error;
-    } /* if (fd < 0) */
+    std = create_stdio(iomod, fd, 1, "<stdout>", encoding, errors);
+    if (std == NULL)
+        goto error;
     PySys_SetObject("__stdout__", std);
     _PySys_SetObjectId(&PyId_stdout, std);
     Py_DECREF(std);
@@ -1219,15 +1220,9 @@ initstdio(void)
 #if 1 /* Disable this if you have trouble debugging bootstrap stuff */
     /* Set sys.stderr, replaces the preliminary stderr */
     fd = fileno(stderr);
-    if (!is_valid_fd(fd)) {
-        std = Py_None;
-        Py_INCREF(std);
-    }
-    else {
-        std = create_stdio(iomod, fd, 1, "<stderr>", encoding, "backslashreplace");
-        if (std == NULL)
-            goto error;
-    } /* if (fd < 0) */
+    std = create_stdio(iomod, fd, 1, "<stderr>", encoding, "backslashreplace");
+    if (std == NULL)
+        goto error;
 
     /* Same as hack above, pre-import stderr's codec to avoid recursion
        when import.c tries to write to stderr in verbose mode. */