Skip to content

Commit 0ef4ffe

Browse files
serhiy-storchakaserhiy-storchaka
authored
pythongh-130163: Fix crashes related to PySys_GetObject() (pythonGH-130503)
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 2dad1e0 commit 0ef4ffe

23 files changed

+509
-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 *);

‎Lib/test/test_builtin.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1710,6 +1710,29 @@ def test_input(self):
17101710
sys.stdout = savestdout
17111711
fp.close()
17121712

1713+
def test_input_gh130163(self):
1714+
class X(io.StringIO):
1715+
def __getattribute__(self, name):
1716+
nonlocal patch
1717+
if patch:
1718+
patch = False
1719+
sys.stdout = X()
1720+
sys.stderr = X()
1721+
sys.stdin = X('input\n')
1722+
support.gc_collect()
1723+
return io.StringIO.__getattribute__(self, name)
1724+
1725+
with (support.swap_attr(sys, 'stdout', None),
1726+
support.swap_attr(sys, 'stderr', None),
1727+
support.swap_attr(sys, 'stdin', None)):
1728+
patch = False
1729+
# the only references:
1730+
sys.stdout = X()
1731+
sys.stderr = X()
1732+
sys.stdin = X('input\n')
1733+
patch = True
1734+
input() # should not crash
1735+
17131736
# test_int(): see test_int.py for tests of built-in function int().
17141737

17151738
def test_repr(self):

‎Lib/test/test_print.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,17 @@ def flush(self):
129129
raise RuntimeError
130130
self.assertRaises(RuntimeError, print, 1, file=noflush(), flush=True)
131131

132+
def test_gh130163(self):
133+
class X:
134+
def __str__(self):
135+
sys.stdout = StringIO()
136+
support.gc_collect()
137+
return 'foo'
138+
139+
with support.swap_attr(sys, 'stdout', None):
140+
sys.stdout = StringIO() # the only reference
141+
print(X()) # should not crash
142+
132143

133144
class TestPy2MigrationHint(unittest.TestCase):
134145
"""Test that correct hint is produced analogous to Python3 syntax,

‎Lib/test/test_sys.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import codecs
33
import _datetime
44
import gc
5+
import io
56
import locale
67
import operator
78
import os
@@ -80,6 +81,18 @@ def baddisplayhook(obj):
8081
code = compile("42", "<string>", "single")
8182
self.assertRaises(ValueError, eval, code)
8283

84+
def test_gh130163(self):
85+
class X:
86+
def __repr__(self):
87+
sys.stdout = io.StringIO()
88+
support.gc_collect()
89+
return 'foo'
90+
91+
with support.swap_attr(sys, 'stdout', None):
92+
sys.stdout = io.StringIO() # the only reference
93+
sys.displayhook(X()) # should not crash
94+
95+
8396
class ActiveExceptionTests(unittest.TestCase):
8497
def test_exc_info_no_exception(self):
8598
self.assertEqual(sys.exc_info(), (None, None, None))

‎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 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

@@ -2249,9 +2249,12 @@ thread_excepthook(PyObject *module, PyObject *args)
22492249
PyObject *exc_tb = PyStructSequence_GET_ITEM(args, 2);
22502250
PyObject *thread = PyStructSequence_GET_ITEM(args, 3);
22512251

2252-
PyThreadState *tstate = _PyThreadState_GET();
2253-
PyObject *file = _PySys_GetAttr(tstate, &_Py_ID(stderr));
2252+
PyObject *file;
2253+
if (_PySys_GetOptionalAttr( &_Py_ID(stderr), &file) < 0) {
2254+
return NULL;
2255+
}
22542256
if (file == NULL || file == Py_None) {
2257+
Py_XDECREF(file);
22552258
if (thread == Py_None) {
22562259
/* do nothing if sys.stderr is None and thread is None */
22572260
Py_RETURN_NONE;
@@ -2268,9 +2271,6 @@ thread_excepthook(PyObject *module, PyObject *args)
22682271
Py_RETURN_NONE;
22692272
}
22702273
}
2271-
else {
2272-
Py_INCREF(file);
2273-
}
22742274

22752275
int res = thread_excepthook_file(file, exc_type, exc_value, exc_tb,
22762276
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);

0 commit comments

Comments
 (0)