Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-107913: Fix possible losses of OSError error codes #107930

Merged
merged 4 commits into from
Aug 26, 2023
Merged

gh-107913: Fix possible losses of OSError error codes #107930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
29f6f36
gh-107913: Fix possible losses of OSError error codes
serhiy-storchaka Aug 13, 2023
0485db3
Merge branch 'main' into use-PyErr_SetFromErrno
serhiy-storchaka Aug 26, 2023
ba06d0a
Address review comments.
serhiy-storchaka Aug 26, 2023
695dbdb
Add missed "return NULL".
serhiy-storchaka Aug 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Library/2023-08-14-11-18-13.gh-issue-107913.4ooY6i.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 1 addition & 1 deletion Modules/_cursesmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 6 additions & 6 deletions Modules/_io/fileio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fd_is_own = 1; changes the code in the error label, it's unclear to me if your change is correct or not.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It only has effect if self->fd >= 0.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see. This change is non-trivial, I preferred to double check.

}
else {
PyObject *fdobj;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion Modules/_io/winconsoleio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
2 changes: 1 addition & 1 deletion Modules/_localemodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
9 changes: 5 additions & 4 deletions Modules/_multiprocessing/semaphore.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions Modules/_ssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this function related to errno? Or is it specific to ssl and it leaves errno unchanged?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not know, but it is better to be on safe side.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'm fine with this change.

}
else {
_setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
Expand All @@ -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__);
Expand Down Expand Up @@ -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__);
Expand Down Expand Up @@ -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__);
Expand Down
3 changes: 1 addition & 2 deletions Modules/faulthandler.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 3 additions & 2 deletions Modules/fcntlmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion Modules/getpath.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 2 additions & 0 deletions Modules/mmapmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
3 changes: 2 additions & 1 deletion Modules/overlapped.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading