Skip to content

Commit e76228a

Browse files
serhiy-storchakaserhiy-storchaka
committed
gh-108512: Fix possible crashes related to PySys_GetObject() in free-threaded build
The use of PySys_GetObject() and _PySys_GetAttr(), which return a borrowed reference, has been replaced by using one of the following functions, which return a strong reference and distinguish a missing attribute from an error: _PySys_GetOptionalAttr(), _PySys_GetOptionalAttrString(), _PySys_GetRequiredAttr(), and _PySys_GetRequiredAttrString().
1 parent de13293 commit e76228a

File tree

20 files changed

+462
-209
lines changed

20 files changed

+462
-209
lines changed

‎Include/internal/pycore_sysmodule.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@ extern "C" {
88
# error "this header requires Py_BUILD_CORE define"
99
#endif
1010

11-
// Export for '_pickle' shared extension
12-
PyAPI_FUNC(PyObject*) _PySys_GetAttr(PyThreadState *tstate, PyObject *name);
11+
PyAPI_FUNC(int) _PySys_GetOptionalAttr(PyObject *, PyObject **);
12+
PyAPI_FUNC(int) _PySys_GetOptionalAttrString(const char *, PyObject **);
13+
PyAPI_FUNC(PyObject *) _PySys_GetRequiredAttr(PyObject *);
14+
PyAPI_FUNC(PyObject *) _PySys_GetRequiredAttrString(const char *);
1315

1416
// Export for '_pickle' shared extension
1517
PyAPI_FUNC(size_t) _PySys_GetSizeOf(PyObject *);

‎Misc/NEWS.d/next/Core_and_Builtins/2025-02-24-14-25-36.gh-issue-130163.rGpc9v.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix possible crashes in a free-threaded debug build related to concurrent
2+
change and use of the :mod:`sys` module attributes.

‎Modules/_cursesmodule.c‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ static const char PyCursesVersion[] = "2.2";
108108
#include "pycore_capsule.h" // _PyCapsule_SetTraverse()
109109
#include "pycore_long.h" // _PyLong_GetZero()
110110
#include "pycore_structseq.h" // _PyStructSequence_NewType()
111+
#include "pycore_sysmodule.h" // _PySys_GetOptionalAttrString()
111112

112113
#ifdef __hpux
113114
#define STRICT_SYSV_CURSES
@@ -3542,16 +3543,19 @@ _curses_setupterm_impl(PyObject *module, const char *term, int fd)
35423543
if (fd == -1) {
35433544
PyObject* sys_stdout;
35443545

3545-
sys_stdout = PySys_GetObject("stdout");
3546+
if (_PySys_GetOptionalAttrString("stdout", &sys_stdout) < 0) {
3547+
return NULL;
3548+
}
35463549

35473550
if (sys_stdout == NULL || sys_stdout == Py_None) {
35483551
cursesmodule_state *state = get_cursesmodule_state(module);
35493552
PyErr_SetString(state->error, "lost sys.stdout");
3553+
Py_XDECREF(sys_stdout);
35503554
return NULL;
35513555
}
35523556

35533557
fd = PyObject_AsFileDescriptor(sys_stdout);
3554-
3558+
Py_DECREF(sys_stdout);
35553559
if (fd == -1) {
35563560
return NULL;
35573561
}

‎Modules/_pickle.c‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
#include "pycore_pystate.h" // _PyThreadState_GET()
2020
#include "pycore_runtime.h" // _Py_ID()
2121
#include "pycore_setobject.h" // _PySet_NextEntry()
22-
#include "pycore_sysmodule.h" // _PySys_GetAttr()
22+
#include "pycore_sysmodule.h" // _PySys_GetSizeOf()
2323

2424
#include <stdlib.h> // strtol()
2525

@@ -1910,10 +1910,8 @@ whichmodule(PickleState *st, PyObject *global, PyObject *global_name, PyObject *
19101910
__module__ can be None. If it is so, then search sys.modules for
19111911
the module of global. */
19121912
Py_CLEAR(module_name);
1913-
PyThreadState *tstate = _PyThreadState_GET();
1914-
modules = _PySys_GetAttr(tstate, &_Py_ID(modules));
1913+
modules = _PySys_GetRequiredAttr(&_Py_ID(modules));
19151914
if (modules == NULL) {
1916-
PyErr_SetString(PyExc_RuntimeError, "unable to get sys.modules");
19171915
return NULL;
19181916
}
19191917
if (PyDict_CheckExact(modules)) {
@@ -1923,41 +1921,48 @@ whichmodule(PickleState *st, PyObject *global, PyObject *global_name, PyObject *
19231921
Py_INCREF(module);
19241922
if (_checkmodule(module_name, module, global, dotted_path) == 0) {
19251923
Py_DECREF(module);
1924+
Py_DECREF(modules);
19261925
return module_name;
19271926
}
19281927
Py_DECREF(module);
19291928
Py_DECREF(module_name);
19301929
if (PyErr_Occurred()) {
1930+
Py_DECREF(modules);
19311931
return NULL;
19321932
}
19331933
}
19341934
}
19351935
else {
19361936
PyObject *iterator = PyObject_GetIter(modules);
19371937
if (iterator == NULL) {
1938+
Py_DECREF(modules);
19381939
return NULL;
19391940
}
19401941
while ((module_name = PyIter_Next(iterator))) {
19411942
module = PyObject_GetItem(modules, module_name);
19421943
if (module == NULL) {
19431944
Py_DECREF(module_name);
19441945
Py_DECREF(iterator);
1946+
Py_DECREF(modules);
19451947
return NULL;
19461948
}
19471949
if (_checkmodule(module_name, module, global, dotted_path) == 0) {
19481950
Py_DECREF(module);
19491951
Py_DECREF(iterator);
1952+
Py_DECREF(modules);
19501953
return module_name;
19511954
}
19521955
Py_DECREF(module);
19531956
Py_DECREF(module_name);
19541957
if (PyErr_Occurred()) {
19551958
Py_DECREF(iterator);
1959+
Py_DECREF(modules);
19561960
return NULL;
19571961
}
19581962
}
19591963
Py_DECREF(iterator);
19601964
}
1965+
Py_DECREF(modules);
19611966
if (PyErr_Occurred()) {
19621967
return NULL;
19631968
}

‎Modules/_threadmodule.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
#include "pycore_modsupport.h" // _PyArg_NoKeywords()
99
#include "pycore_pylifecycle.h"
1010
#include "pycore_pystate.h" // _PyThreadState_SetCurrent()
11-
#include "pycore_sysmodule.h" // _PySys_GetAttr()
11+
#include "pycore_sysmodule.h" // _PySys_GetOptionalAttr()
1212
#include "pycore_time.h" // _PyTime_FromSeconds()
1313
#include "pycore_weakref.h" // _PyWeakref_GET_REF()
1414

@@ -2242,9 +2242,12 @@ thread_excepthook(PyObject *module, PyObject *args)
22422242
PyObject *exc_tb = PyStructSequence_GET_ITEM(args, 2);
22432243
PyObject *thread = PyStructSequence_GET_ITEM(args, 3);
22442244

2245-
PyThreadState *tstate = _PyThreadState_GET();
2246-
PyObject *file = _PySys_GetAttr(tstate, &_Py_ID(stderr));
2245+
PyObject *file;
2246+
if (_PySys_GetOptionalAttr( &_Py_ID(stderr), &file) < 0) {
2247+
return NULL;
2248+
}
22472249
if (file == NULL || file == Py_None) {
2250+
Py_XDECREF(file);
22482251
if (thread == Py_None) {
22492252
/* do nothing if sys.stderr is None and thread is None */
22502253
Py_RETURN_NONE;
@@ -2261,9 +2264,6 @@ thread_excepthook(PyObject *module, PyObject *args)
22612264
Py_RETURN_NONE;
22622265
}
22632266
}
2264-
else {
2265-
Py_INCREF(file);
2266-
}
22672267

22682268
int res = thread_excepthook_file(file, exc_type, exc_value, exc_tb,
22692269
thread);

‎Modules/_tkinter.c‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ Copyright (C) 1994 Steen Lumholt.
3131
#endif
3232

3333
#include "pycore_long.h" // _PyLong_IsNegative()
34+
#include "pycore_sysmodule.h" // _PySys_GetOptionalAttrString()
3435

3536
#ifdef MS_WINDOWS
3637
# include <windows.h>
@@ -142,18 +143,21 @@ _get_tcl_lib_path(void)
142143
if (already_checked == 0) {
143144
struct stat stat_buf;
144145
int stat_return_value;
146+
PyObject *prefix;
145147

146-
PyObject *prefix = PySys_GetObject("base_prefix"); // borrowed reference
148+
(void) _PySys_GetOptionalAttrString("base_prefix", &prefix);
147149
if (prefix == NULL) {
148150
return NULL;
149151
}
150152

151153
/* Check expected location for an installed Python first */
152154
tcl_library_path = PyUnicode_FromString("\\tcl\\tcl" TCL_VERSION);
153155
if (tcl_library_path == NULL) {
156+
Py_DECREF(prefix);
154157
return NULL;
155158
}
156159
tcl_library_path = PyUnicode_Concat(prefix, tcl_library_path);
160+
Py_DECREF(prefix);
157161
if (tcl_library_path == NULL) {
158162
return NULL;
159163
}
@@ -3516,9 +3520,10 @@ PyInit__tkinter(void)
35163520

35173521
/* This helps the dynamic loader; in Unicode aware Tcl versions
35183522
it also helps Tcl find its encodings. */
3519-
uexe = PySys_GetObject("executable"); // borrowed reference
3523+
(void) _PySys_GetOptionalAttrString("executable", &uexe);
35203524
if (uexe && PyUnicode_Check(uexe)) { // sys.executable can be None
35213525
cexe = PyUnicode_EncodeFSDefault(uexe);
3526+
Py_DECREF(uexe);
35223527
if (cexe) {
35233528
#ifdef MS_WINDOWS
35243529
int set_var = 0;
@@ -3531,12 +3536,14 @@ PyInit__tkinter(void)
35313536
if (!ret && GetLastError() == ERROR_ENVVAR_NOT_FOUND) {
35323537
str_path = _get_tcl_lib_path();
35333538
if (str_path == NULL && PyErr_Occurred()) {
3539+
Py_DECREF(cexe);
35343540
Py_DECREF(m);
35353541
return NULL;
35363542
}
35373543
if (str_path != NULL) {
35383544
wcs_path = PyUnicode_AsWideCharString(str_path, NULL);
35393545
if (wcs_path == NULL) {
3546+
Py_DECREF(cexe);
35403547
Py_DECREF(m);
35413548
return NULL;
35423549
}
@@ -3557,6 +3564,9 @@ PyInit__tkinter(void)
35573564
}
35583565
Py_XDECREF(cexe);
35593566
}
3567+
else {
3568+
Py_XDECREF(uexe);
3569+
}
35603570

35613571
if (PyErr_Occurred()) {
35623572
Py_DECREF(m);

‎Modules/faulthandler.c‎

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
#include "pycore_pyerrors.h" // _Py_DumpExtensionModules
55
#include "pycore_pystate.h" // _PyThreadState_GET()
66
#include "pycore_signal.h" // Py_NSIG
7-
#include "pycore_sysmodule.h" // _PySys_GetAttr()
7+
#include "pycore_sysmodule.h" // _PySys_GetRequiredAttr()
88
#include "pycore_time.h" // _PyTime_FromSecondsObject()
99
#include "pycore_traceback.h" // _Py_DumpTracebackThreads
1010

@@ -112,14 +112,13 @@ faulthandler_get_fileno(PyObject **file_ptr)
112112
PyObject *file = *file_ptr;
113113

114114
if (file == NULL || file == Py_None) {
115-
PyThreadState *tstate = _PyThreadState_GET();
116-
file = _PySys_GetAttr(tstate, &_Py_ID(stderr));
115+
file = _PySys_GetRequiredAttr(&_Py_ID(stderr));
117116
if (file == NULL) {
118-
PyErr_SetString(PyExc_RuntimeError, "unable to get sys.stderr");
119117
return -1;
120118
}
121119
if (file == Py_None) {
122120
PyErr_SetString(PyExc_RuntimeError, "sys.stderr is None");
121+
Py_DECREF(file);
123122
return -1;
124123
}
125124
}
@@ -142,10 +141,15 @@ faulthandler_get_fileno(PyObject **file_ptr)
142141
*file_ptr = NULL;
143142
return fd;
144143
}
144+
else {
145+
Py_INCREF(file);
146+
}
145147

146148
result = PyObject_CallMethodNoArgs(file, &_Py_ID(fileno));
147-
if (result == NULL)
149+
if (result == NULL) {
150+
Py_DECREF(file);
148151
return -1;
152+
}
149153

150154
fd = -1;
151155
if (PyLong_Check(result)) {
@@ -158,6 +162,7 @@ faulthandler_get_fileno(PyObject **file_ptr)
158162
if (fd == -1) {
159163
PyErr_SetString(PyExc_RuntimeError,
160164
"file.fileno() is not a valid file descriptor");
165+
Py_DECREF(file);
161166
return -1;
162167
}
163168

@@ -240,8 +245,10 @@ faulthandler_dump_traceback_py(PyObject *self,
240245
return NULL;
241246

242247
tstate = get_thread_state();
243-
if (tstate == NULL)
248+
if (tstate == NULL) {
249+
Py_XDECREF(file);
244250
return NULL;
251+
}
245252

246253
if (all_threads) {
247254
PyInterpreterState *interp = _PyInterpreterState_GET();
@@ -252,12 +259,14 @@ faulthandler_dump_traceback_py(PyObject *self,
252259
_PyEval_StartTheWorld(interp);
253260
if (errmsg != NULL) {
254261
PyErr_SetString(PyExc_RuntimeError, errmsg);
262+
Py_XDECREF(file);
255263
return NULL;
256264
}
257265
}
258266
else {
259267
_Py_DumpTraceback(fd, tstate);
260268
}
269+
Py_XDECREF(file);
261270

262271
if (PyErr_CheckSignals())
263272
return NULL;
@@ -540,10 +549,11 @@ faulthandler_py_enable(PyObject *self, PyObject *args, PyObject *kwargs)
540549
return NULL;
541550

542551
tstate = get_thread_state();
543-
if (tstate == NULL)
552+
if (tstate == NULL) {
553+
Py_XDECREF(file);
544554
return NULL;
555+
}
545556

546-
Py_XINCREF(file);
547557
Py_XSETREF(fatal_error.file, file);
548558
fatal_error.fd = fd;
549559
fatal_error.all_threads = all_threads;
@@ -733,12 +743,14 @@ faulthandler_dump_traceback_later(PyObject *self,
733743
if (!thread.running) {
734744
thread.running = PyThread_allocate_lock();
735745
if (!thread.running) {
746+
Py_XDECREF(file);
736747
return PyErr_NoMemory();
737748
}
738749
}
739750
if (!thread.cancel_event) {
740751
thread.cancel_event = PyThread_allocate_lock();
741752
if (!thread.cancel_event || !thread.running) {
753+
Py_XDECREF(file);
742754
return PyErr_NoMemory();
743755
}
744756

@@ -750,14 +762,14 @@ faulthandler_dump_traceback_later(PyObject *self,
750762
/* format the timeout */
751763
header = format_timeout(timeout_us);
752764
if (header == NULL) {
765+
Py_XDECREF(file);
753766
return PyErr_NoMemory();
754767
}
755768
header_len = strlen(header);
756769

757770
/* Cancel previous thread, if running */
758771
cancel_dump_traceback_later();
759772

760-
Py_XINCREF(file);
761773
Py_XSETREF(thread.file, file);
762774
thread.fd = fd;
763775
/* the downcast is safe: we check that 0 < timeout_us < PY_TIMEOUT_MAX */
@@ -919,28 +931,31 @@ faulthandler_register_py(PyObject *self,
919931

920932
if (user_signals == NULL) {
921933
user_signals = PyMem_Calloc(Py_NSIG, sizeof(user_signal_t));
922-
if (user_signals == NULL)
934+
if (user_signals == NULL) {
935+
Py_XDECREF(file);
923936
return PyErr_NoMemory();
937+
}
924938
}
925939
user = &user_signals[signum];
926940

927941
if (!user->enabled) {
928942
#ifdef FAULTHANDLER_USE_ALT_STACK
929943
if (faulthandler_allocate_stack() < 0) {
944+
Py_XDECREF(file);
930945
return NULL;
931946
}
932947
#endif
933948

934949
err = faulthandler_register(signum, chain, &previous);
935950
if (err) {
936951
PyErr_SetFromErrno(PyExc_OSError);
952+
Py_XDECREF(file);
937953
return NULL;
938954
}
939955

940956
user->previous = previous;
941957
}
942958

943-
Py_XINCREF(file);
944959
Py_XSETREF(user->file, file);
945960
user->fd = fd;
946961
user->all_threads = all_threads;

0 commit comments

Comments
 (0)