gh-107913: Fix possible losses of OSError error codes (GH-107930)

serhiy-storchaka · web-flow · commit 2b15536fa94d · 2023-08-27T00:35:06.000+03:00
Functions like PyErr_SetFromErrno() and SetFromWindowsErr() should be
called immediately after using the C API which sets errno or the Windows
error code.
diff --git a/Misc/NEWS.d/next/Library/2023-08-14-11-18-13.gh-issue-107913.4ooY6i.rst b/Misc/NEWS.d/next/Library/2023-08-14-11-18-13.gh-issue-107913.4ooY6i.rst
@@ -0,0 +1,3 @@
+Fix possible losses of ``errno`` and ``winerror`` values in :exc:`OSError`
+exceptions if they were cleared or modified by the cleanup code before
+creating the exception object.
diff --git a/Modules/_cursesmodule.c b/Modules/_cursesmodule.c
@@ -3069,8 +3069,8 @@ _curses_getwin(PyObject *module, PyObject *file)
     }
     datalen = PyBytes_GET_SIZE(data);
     if (fwrite(PyBytes_AS_STRING(data), 1, datalen, fp) != datalen) {
-        Py_DECREF(data);
         PyErr_SetFromErrno(PyExc_OSError);
+        Py_DECREF(data);
         goto error;
     }
     Py_DECREF(data);
diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c
@@ -393,6 +393,11 @@ _io_FileIO___init___impl(fileio *self, PyObject *nameobj, const char *mode,
 
             if (async_err)
                 goto error;
+
+            if (self->fd < 0) {
+                PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, nameobj);
+                goto error;
+            }
         }
         else {
             PyObject *fdobj;
@@ -424,12 +429,7 @@ _io_FileIO___init___impl(fileio *self, PyObject *nameobj, const char *mode,
                 goto error;
             }
         }
-
         fd_is_own = 1;
-        if (self->fd < 0) {
-            PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, nameobj);
-            goto error;
-        }
 
 #ifndef MS_WINDOWS
         if (_Py_set_inheritable(self->fd, 0, atomic_flag_works) < 0)
@@ -1057,8 +1057,8 @@ _io_FileIO_truncate_impl(fileio *self, PyTypeObject *cls, PyObject *posobj)
     Py_END_ALLOW_THREADS
 
     if (ret != 0) {
-        Py_DECREF(posobj);
         PyErr_SetFromErrno(PyExc_OSError);
+        Py_DECREF(posobj);
         return NULL;
     }
 
diff --git a/Modules/_io/winconsoleio.c b/Modules/_io/winconsoleio.c
@@ -377,8 +377,8 @@ _io__WindowsConsoleIO___init___impl(winconsoleio *self, PyObject *nameobj,
         else
             self->fd = _Py_open_osfhandle_noraise(handle, _O_RDONLY | _O_BINARY);
         if (self->fd < 0) {
-            CloseHandle(handle);
             PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, nameobj);
+            CloseHandle(handle);
             goto error;
         }
     }
diff --git a/Modules/_localemodule.c b/Modules/_localemodule.c
@@ -736,8 +736,8 @@ _locale_bindtextdomain_impl(PyObject *module, const char *domain,
     }
     current_dirname = bindtextdomain(domain, dirname);
     if (current_dirname == NULL) {
-        Py_XDECREF(dirname_bytes);
         PyErr_SetFromErrno(PyExc_OSError);
+        Py_XDECREF(dirname_bytes);
         return NULL;
     }
     result = PyUnicode_DecodeLocale(current_dirname, NULL);
diff --git a/Modules/_multiprocessing/semaphore.c b/Modules/_multiprocessing/semaphore.c
@@ -516,12 +516,12 @@ _multiprocessing_SemLock_impl(PyTypeObject *type, int kind, int value,
     return result;
 
   failure:
-    if (handle != SEM_FAILED)
-        SEM_CLOSE(handle);
-    PyMem_Free(name_copy);
     if (!PyErr_Occurred()) {
         _PyMp_SetError(NULL, MP_STANDARD_ERROR);
     }
+    if (handle != SEM_FAILED)
+        SEM_CLOSE(handle);
+    PyMem_Free(name_copy);
     return NULL;
 }
 
@@ -556,8 +556,9 @@ _multiprocessing_SemLock__rebuild_impl(PyTypeObject *type, SEM_HANDLE handle,
     if (name != NULL) {
         handle = sem_open(name, 0);
         if (handle == SEM_FAILED) {
+            PyErr_SetFromErrno(PyExc_OSError);
             PyMem_Free(name_copy);
-            return PyErr_SetFromErrno(PyExc_OSError);
+            return NULL;
         }
     }
 #endif
diff --git a/Modules/_ssl.c b/Modules/_ssl.c
@@ -3914,8 +3914,8 @@ _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile,
             /* the password callback has already set the error information */
         }
         else if (errno != 0) {
-            ERR_clear_error();
             PyErr_SetFromErrno(PyExc_OSError);
+            ERR_clear_error();
         }
         else {
             _setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
@@ -3935,8 +3935,8 @@ _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile,
             /* the password callback has already set the error information */
         }
         else if (errno != 0) {
-            ERR_clear_error();
             PyErr_SetFromErrno(PyExc_OSError);
+            ERR_clear_error();
         }
         else {
             _setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
@@ -4165,8 +4165,8 @@ _ssl__SSLContext_load_verify_locations_impl(PySSLContext *self,
         PySSL_END_ALLOW_THREADS
         if (r != 1) {
             if (errno != 0) {
-                ERR_clear_error();
                 PyErr_SetFromErrno(PyExc_OSError);
+                ERR_clear_error();
             }
             else {
                 _setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
@@ -4213,8 +4213,8 @@ _ssl__SSLContext_load_dh_params(PySSLContext *self, PyObject *filepath)
     PySSL_END_ALLOW_THREADS
     if (dh == NULL) {
         if (errno != 0) {
-            ERR_clear_error();
             PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, filepath);
+            ERR_clear_error();
         }
         else {
             _setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
diff --git a/Modules/faulthandler.c b/Modules/faulthandler.c
@@ -415,11 +415,10 @@ faulthandler_allocate_stack(void)
 
     int err = sigaltstack(&stack, &old_stack);
     if (err) {
+        PyErr_SetFromErrno(PyExc_OSError);
         /* Release the stack to retry sigaltstack() next time */
         PyMem_Free(stack.ss_sp);
         stack.ss_sp = NULL;
-
-        PyErr_SetFromErrno(PyExc_OSError);
         return -1;
     }
     return 0;
diff --git a/Modules/fcntlmodule.c b/Modules/fcntlmodule.c
@@ -212,11 +212,12 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned int code,
             if (mutate_arg && (len <= IOCTL_BUFSZ)) {
                 memcpy(str, buf, len);
             }
-            PyBuffer_Release(&pstr); /* No further access to str below this point */
             if (ret < 0) {
                 PyErr_SetFromErrno(PyExc_OSError);
+                PyBuffer_Release(&pstr);
                 return NULL;
             }
+            PyBuffer_Release(&pstr);
             if (mutate_arg) {
                 return PyLong_FromLong(ret);
             }
@@ -241,8 +242,8 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned int code,
             ret = ioctl(fd, code, buf);
             Py_END_ALLOW_THREADS
             if (ret < 0) {
-                PyBuffer_Release(&pstr);
                 PyErr_SetFromErrno(PyExc_OSError);
+                PyBuffer_Release(&pstr);
                 return NULL;
             }
             PyBuffer_Release(&pstr);
diff --git a/Modules/getpath.c b/Modules/getpath.c
@@ -342,11 +342,12 @@ getpath_readlines(PyObject *Py_UNUSED(self), PyObject *args)
         return NULL;
     }
     FILE *fp = _Py_wfopen(path, L"rb");
-    PyMem_Free((void *)path);
     if (!fp) {
         PyErr_SetFromErrno(PyExc_OSError);
+        PyMem_Free((void *)path);
         return NULL;
     }
+    PyMem_Free((void *)path);
 
     r = PyList_New(0);
     if (!r) {
diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c
@@ -1356,13 +1356,15 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict)
     m_obj->data = mmap(NULL, map_size, prot, flags, fd, offset);
     Py_END_ALLOW_THREADS
 
+    int saved_errno = errno;
     if (devzero != -1) {
         close(devzero);
     }
 
     if (m_obj->data == (char *)-1) {
         m_obj->data = NULL;
         Py_DECREF(m_obj);
+        errno = saved_errno;
         PyErr_SetFromErrno(PyExc_OSError);
         return NULL;
     }
diff --git a/Modules/overlapped.c b/Modules/overlapped.c
@@ -370,8 +370,9 @@ _overlapped_RegisterWaitWithQueue_impl(PyObject *module, HANDLE Object,
             &NewWaitObject, Object, PostToQueueCallback, pdata, Milliseconds,
             WT_EXECUTEINWAITTHREAD | WT_EXECUTEONLYONCE))
     {
+        SetFromWindowsErr(0);
         PyMem_RawFree(pdata);
-        return SetFromWindowsErr(0);
+        return NULL;
     }
 
     return Py_BuildValue(F_HANDLE, NewWaitObject);
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
diff --git a/Modules/selectmodule.c b/Modules/selectmodule.c
diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c
diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c
diff --git a/Python/fileutils.c b/Python/fileutils.c

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	+Fix possible losses of ``errno`` and ``winerror`` values in :exc:`OSError`
	`2`	`+exceptions if they were cleared or modified by the cleanup code before`
	`3`	`+creating the exception object.`
Original file line number	Diff line number	Diff line change
`@@ -3069,8 +3069,8 @@ _curses_getwin(PyObject module, PyObject file)`
`3069`	`3069`	`}`
`3070`	`3070`	`datalen = PyBytes_GET_SIZE(data);`
`3071`	`3071`	`if (fwrite(PyBytes_AS_STRING(data), 1, datalen, fp) != datalen) {`
`3072`		`- Py_DECREF(data);`
`3073`	`3072`	`PyErr_SetFromErrno(PyExc_OSError);`
	`3073`	`+ Py_DECREF(data);`
`3074`	`3074`	`goto error;`
`3075`	`3075`	`}`
`3076`	`3076`	`Py_DECREF(data);`
Original file line number	Diff line number	Diff line change
`@@ -377,8 +377,8 @@ _io__WindowsConsoleIO___init___impl(winconsoleio self, PyObject nameobj,`
`377`	`377`	`else`
`378`	`378`	`self->fd = _Py_open_osfhandle_noraise(handle, _O_RDONLY \| _O_BINARY);`
`379`	`379`	`if (self->fd < 0) {`
`380`		`- CloseHandle(handle);`
`381`	`380`	`PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, nameobj);`
	`381`	`+ CloseHandle(handle);`
`382`	`382`	`goto error;`
`383`	`383`	`}`
`384`	`384`	`}`
Original file line number	Diff line number	Diff line change
`@@ -736,8 +736,8 @@ _locale_bindtextdomain_impl(PyObject module, const char domain,`
`736`	`736`	`}`
`737`	`737`	`current_dirname = bindtextdomain(domain, dirname);`
`738`	`738`	`if (current_dirname == NULL) {`
`739`		`- Py_XDECREF(dirname_bytes);`
`740`	`739`	`PyErr_SetFromErrno(PyExc_OSError);`
	`740`	`+ Py_XDECREF(dirname_bytes);`
`741`	`741`	`return NULL;`
`742`	`742`	`}`
`743`	`743`	`result = PyUnicode_DecodeLocale(current_dirname, NULL);`
Original file line number	Diff line number	Diff line change
`@@ -516,12 +516,12 @@ _multiprocessing_SemLock_impl(PyTypeObject *type, int kind, int value,`
`516`	`516`	`return result;`
`517`	`517`
`518`	`518`	`failure:`
`519`		`- if (handle != SEM_FAILED)`
`520`		`- SEM_CLOSE(handle);`
`521`		`- PyMem_Free(name_copy);`
`522`	`519`	`if (!PyErr_Occurred()) {`
`523`	`520`	`_PyMp_SetError(NULL, MP_STANDARD_ERROR);`
`524`	`521`	`}`
	`522`	`+ if (handle != SEM_FAILED)`
	`523`	`+ SEM_CLOSE(handle);`
	`524`	`+ PyMem_Free(name_copy);`
`525`	`525`	`return NULL;`
`526`	`526`	`}`
`527`	`527`
`@@ -556,8 +556,9 @@ _multiprocessing_SemLock__rebuild_impl(PyTypeObject *type, SEM_HANDLE handle,`
`556`	`556`	`if (name != NULL) {`
`557`	`557`	`handle = sem_open(name, 0);`
`558`	`558`	`if (handle == SEM_FAILED) {`
	`559`	`+ PyErr_SetFromErrno(PyExc_OSError);`
`559`	`560`	`PyMem_Free(name_copy);`
`560`		`- return PyErr_SetFromErrno(PyExc_OSError);`
	`561`	`+ return NULL;`
`561`	`562`	`}`
`562`	`563`	`}`
`563`	`564`	`#endif`
Original file line number	Diff line number	Diff line change
`@@ -212,11 +212,12 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned int code,`
`212`	`212`	`if (mutate_arg && (len <= IOCTL_BUFSZ)) {`
`213`	`213`	`memcpy(str, buf, len);`
`214`	`214`	`}`
`215`		`- PyBuffer_Release(&pstr); /* No further access to str below this point */`
`216`	`215`	`if (ret < 0) {`
`217`	`216`	`PyErr_SetFromErrno(PyExc_OSError);`
	`217`	`+ PyBuffer_Release(&pstr);`
`218`	`218`	`return NULL;`
`219`	`219`	`}`
	`220`	`+ PyBuffer_Release(&pstr);`
`220`	`221`	`if (mutate_arg) {`
`221`	`222`	`return PyLong_FromLong(ret);`
`222`	`223`	`}`
`@@ -241,8 +242,8 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned int code,`
`241`	`242`	`ret = ioctl(fd, code, buf);`
`242`	`243`	`Py_END_ALLOW_THREADS`
`243`	`244`	`if (ret < 0) {`
`244`		`- PyBuffer_Release(&pstr);`
`245`	`245`	`PyErr_SetFromErrno(PyExc_OSError);`
	`246`	`+ PyBuffer_Release(&pstr);`
`246`	`247`	`return NULL;`
`247`	`248`	`}`
`248`	`249`	`PyBuffer_Release(&pstr);`
Original file line number	Diff line number	Diff line change
`@@ -342,11 +342,12 @@ getpath_readlines(PyObject Py_UNUSED(self), PyObject args)`
`342`	`342`	`return NULL;`
`343`	`343`	`}`
`344`	`344`	`FILE *fp = _Py_wfopen(path, L"rb");`
`345`		`- PyMem_Free((void *)path);`
`346`	`345`	`if (!fp) {`
`347`	`346`	`PyErr_SetFromErrno(PyExc_OSError);`
	`347`	`+ PyMem_Free((void *)path);`
`348`	`348`	`return NULL;`
`349`	`349`	`}`
	`350`	`+ PyMem_Free((void *)path);`
`350`	`351`
`351`	`352`	`r = PyList_New(0);`
`352`	`353`	`if (!r) {`