[3.11] gh-107913: Fix possible losses of OSError error codes (GH-107930) (GH-108524)

serhiy-storchaka · web-flow · commit b9fc5363997c · 2023-08-27T12:18:58.000Z
Functions like PyErr_SetFromErrno() and SetFromWindowsErr() should be called immediately after using the C API which sets errno or the Windows error code. (cherry picked from commit 2b15536)
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
@@ -3075,8 +3075,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
@@ -389,6 +389,11 @@ _Py_COMP_DIAG_POP
 
             if (async_err)
                 goto error;
+
+            if (self->fd < 0) {
+                PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, nameobj);
+                goto error;
+            }
         }
         else {
             PyObject *fdobj;
@@ -420,12 +425,7 @@ _Py_COMP_DIAG_POP
                 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)
@@ -1044,8 +1044,8 @@ _io_FileIO_truncate_impl(fileio *self, 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
@@ -368,8 +368,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
@@ -735,8 +735,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
@@ -3898,8 +3898,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__);
@@ -3919,8 +3919,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__);
@@ -4147,8 +4147,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__);
@@ -4195,8 +4195,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
@@ -469,11 +469,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
@@ -208,11 +208,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);
             }
@@ -237,8 +238,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
@@ -347,11 +347,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
@@ -1366,13 +1366,15 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict)
                        prot, flags,
                        fd, offset);
 
+    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
@@ -365,8 +365,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
`@@ -3075,8 +3075,8 @@ _curses_getwin(PyObject module, PyObject file)`
`3075`	`3075`	`}`
`3076`	`3076`	`datalen = PyBytes_GET_SIZE(data);`
`3077`	`3077`	`if (fwrite(PyBytes_AS_STRING(data), 1, datalen, fp) != datalen) {`
`3078`		`- Py_DECREF(data);`
`3079`	`3078`	`PyErr_SetFromErrno(PyExc_OSError);`
	`3079`	`+ Py_DECREF(data);`
`3080`	`3080`	`goto error;`
`3081`	`3081`	`}`
`3082`	`3082`	`Py_DECREF(data);`
Original file line number	Diff line number	Diff line change
`@@ -368,8 +368,8 @@ _io__WindowsConsoleIO___init___impl(winconsoleio self, PyObject nameobj,`
`368`	`368`	`else`
`369`	`369`	`self->fd = _Py_open_osfhandle_noraise(handle, _O_RDONLY \| _O_BINARY);`
`370`	`370`	`if (self->fd < 0) {`
`371`		`- CloseHandle(handle);`
`372`	`371`	`PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, nameobj);`
	`372`	`+ CloseHandle(handle);`
`373`	`373`	`goto error;`
`374`	`374`	`}`
`375`	`375`	`}`
Original file line number	Diff line number	Diff line change
`@@ -735,8 +735,8 @@ _locale_bindtextdomain_impl(PyObject module, const char domain,`
`735`	`735`	`}`
`736`	`736`	`current_dirname = bindtextdomain(domain, dirname);`
`737`	`737`	`if (current_dirname == NULL) {`
`738`		`- Py_XDECREF(dirname_bytes);`
`739`	`738`	`PyErr_SetFromErrno(PyExc_OSError);`
	`739`	`+ Py_XDECREF(dirname_bytes);`
`740`	`740`	`return NULL;`
`741`	`741`	`}`
`742`	`742`	`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
`@@ -208,11 +208,12 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned int code,`
`208`	`208`	`if (mutate_arg && (len <= IOCTL_BUFSZ)) {`
`209`	`209`	`memcpy(str, buf, len);`
`210`	`210`	`}`
`211`		`- PyBuffer_Release(&pstr); /* No further access to str below this point */`
`212`	`211`	`if (ret < 0) {`
`213`	`212`	`PyErr_SetFromErrno(PyExc_OSError);`
	`213`	`+ PyBuffer_Release(&pstr);`
`214`	`214`	`return NULL;`
`215`	`215`	`}`
	`216`	`+ PyBuffer_Release(&pstr);`
`216`	`217`	`if (mutate_arg) {`
`217`	`218`	`return PyLong_FromLong(ret);`
`218`	`219`	`}`
`@@ -237,8 +238,8 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned int code,`
`237`	`238`	`ret = ioctl(fd, code, buf);`
`238`	`239`	`Py_END_ALLOW_THREADS`
`239`	`240`	`if (ret < 0) {`
`240`		`- PyBuffer_Release(&pstr);`
`241`	`241`	`PyErr_SetFromErrno(PyExc_OSError);`
	`242`	`+ PyBuffer_Release(&pstr);`
`242`	`243`	`return NULL;`
`243`	`244`	`}`
`244`	`245`	`PyBuffer_Release(&pstr);`
Original file line number	Diff line number	Diff line change
`@@ -347,11 +347,12 @@ getpath_readlines(PyObject Py_UNUSED(self), PyObject args)`
`347`	`347`	`return NULL;`
`348`	`348`	`}`
`349`	`349`	`FILE *fp = _Py_wfopen(path, L"rb");`
`350`		`- PyMem_Free((void *)path);`
`351`	`350`	`if (!fp) {`
`352`	`351`	`PyErr_SetFromErrno(PyExc_OSError);`
	`352`	`+ PyMem_Free((void *)path);`
`353`	`353`	`return NULL;`
`354`	`354`	`}`
	`355`	`+ PyMem_Free((void *)path);`
`355`	`356`
`356`	`357`	`r = PyList_New(0);`
`357`	`358`	`if (!r) {`
Original file line number	Diff line number	Diff line change
`@@ -1366,13 +1366,15 @@ new_mmap_object(PyTypeObject type, PyObject args, PyObject *kwdict)`
`1366`	`1366`	`prot, flags,`
`1367`	`1367`	`fd, offset);`
`1368`	`1368`
	`1369`	`+ int saved_errno = errno;`
`1369`	`1370`	`if (devzero != -1) {`
`1370`	`1371`	`close(devzero);`
`1371`	`1372`	`}`
`1372`	`1373`
`1373`	`1374`	`if (m_obj->data == (char *)-1) {`
`1374`	`1375`	`m_obj->data = NULL;`
`1375`	`1376`	`Py_DECREF(m_obj);`
	`1377`	`+ errno = saved_errno;`
`1376`	`1378`	`PyErr_SetFromErrno(PyExc_OSError);`
`1377`	`1379`	`return NULL;`
`1378`	`1380`	`}`
Original file line number	Diff line number	Diff line change
`@@ -365,8 +365,9 @@ _overlapped_RegisterWaitWithQueue_impl(PyObject *module, HANDLE Object,`
`365`	`365`	`&NewWaitObject, Object, PostToQueueCallback, pdata, Milliseconds,`
`366`	`366`	`WT_EXECUTEINWAITTHREAD \| WT_EXECUTEONLYONCE))`
`367`	`367`	`{`
	`368`	`+ SetFromWindowsErr(0);`
`368`	`369`	`PyMem_RawFree(pdata);`
`369`		`- return SetFromWindowsErr(0);`
	`370`	`+ return NULL;`
`370`	`371`	`}`
`371`	`372`
`372`	`373`	`return Py_BuildValue(F_HANDLE, NewWaitObject);`