diff --git a/Doc/library/profiling.sampling.rst b/Doc/library/profiling.sampling.rst index 9bc58b4d1bc976..4ec594c041d3f5 100644 --- a/Doc/library/profiling.sampling.rst +++ b/Doc/library/profiling.sampling.rst @@ -878,9 +878,9 @@ interesting functions that highlights: Use :option:`--no-summary` to suppress both the legend and summary sections. -To save pstats output to a file instead of stdout:: +To save pstats output to a binary file instead of stdout:: - python -m profiling.sampling run -o profile.txt script.py + python -m profiling.sampling run -o profile.pstats script.py The pstats format supports several options for controlling the display. The :option:`--sort` option determines the column used for ordering results:: @@ -1455,7 +1455,9 @@ Output options .. option:: --pstats - Generate text statistics output. This is the default. + Generate pstats statistics. This is the default. + When written to stdout, the output is a text table; with :option:`-o`, + it is a binary pstats file. .. option:: --collapsed @@ -1486,8 +1488,9 @@ Output options .. option:: -o , --output Output file or directory path. Default behavior varies by format: - :option:`--pstats` writes to stdout, while other formats generate a file - named ``_.`` (for example, ``flamegraph_12345.html``). + :option:`--pstats` prints a text table to stdout, while ``-o`` writes a + binary pstats file. Other formats generate a file named + ``_.`` (for example, ``flamegraph_12345.html``). :option:`--heatmap` creates a directory named ``heatmap_``. .. option:: --browser diff --git a/Lib/profiling/sampling/cli.py b/Lib/profiling/sampling/cli.py index c0dcda46fc29d3..f4b31aad45b922 100644 --- a/Lib/profiling/sampling/cli.py +++ b/Lib/profiling/sampling/cli.py @@ -489,8 +489,8 @@ def _add_format_options(parser, include_compression=True, include_binary=True): "-o", "--output", dest="outfile", - help="Output path (default: stdout for pstats, auto-generated for others). " - "For heatmap: directory name (default: heatmap_PID)", + help="Output path (default: stdout for pstats text; with -o, pstats is binary). " + "Auto-generated for other formats. For heatmap: directory name (default: heatmap_PID)", ) output_group.add_argument( "--browser", diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index 4e3468bfcde9c3..490b34572849e3 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -4261,6 +4261,35 @@ def check_array(arr): # 2-D, non-contiguous check_array(arr[::2]) + def test_concurrent_mutation_in_buffer_with_bytearray(self): + def factory(): + s = b"a" * 16 + return bytearray(s), s + self.do_test_concurrent_mutation_in_buffer_callback(factory) + + def test_concurrent_mutation_in_buffer_with_memoryview(self): + def factory(): + obj = memoryview(b"a" * 32)[10:26] + sub = b"a" * len(obj) + return obj, sub + self.do_test_concurrent_mutation_in_buffer_callback(factory) + + def do_test_concurrent_mutation_in_buffer_callback(self, factory): + # See: https://github.com/python/cpython/issues/143308. + class R: + def __bool__(self): + buf.release() + return True + + for proto in range(5, pickle.HIGHEST_PROTOCOL + 1): + obj, sub = factory() + buf = pickle.PickleBuffer(obj) + buffer_callback = lambda _: R() + + with self.subTest(proto=proto, obj=obj, sub=sub): + res = self.dumps(buf, proto, buffer_callback=buffer_callback) + self.assertIn(sub, res) + def test_evil_class_mutating_dict(self): # https://github.com/python/cpython/issues/92930 from random import getrandbits diff --git a/Lib/test/test_os/test_os.py b/Lib/test/test_os/test_os.py index ddb8a63095bce5..67ab945101149d 100644 --- a/Lib/test/test_os/test_os.py +++ b/Lib/test/test_os/test_os.py @@ -2624,6 +2624,40 @@ def test_execve_invalid_env(self): with self.assertRaises(ValueError): os.execve(args[0], args, newenv) + # See https://github.com/python/cpython/issues/137934 and the other + # related issues for the reason why we cannot test this on Windows. + @unittest.skipIf(os.name == "nt", "POSIX-specific test") + def test_execve_env_concurrent_mutation_with_fspath_posix(self): + # Prevent crash when mutating environment during parsing. + # Regression test for https://github.com/python/cpython/issues/143309. + + message = "hello from execve" + code = """ + import os, sys + + class MyPath: + def __fspath__(self): + mutated.clear() + return b"pwn" + + mutated = KEYS = VALUES = [MyPath()] + + class MyEnv: + def __getitem__(self): raise RuntimeError("must not be called") + def __len__(self): return 1 + def keys(self): return KEYS + def values(self): return VALUES + + args = [sys.executable, '-c', "print({message!r})"] + os.execve(args[0], args, MyEnv()) + """.format(message=message) + + # Use '__cleanenv' to signal to assert_python_ok() not + # to do a copy of os.environ on its own. + rc, out, _ = assert_python_ok('-c', code, __cleanenv=True) + self.assertEqual(rc, 0) + self.assertIn(bytes(message, "ascii"), out) + @unittest.skipUnless(sys.platform == "win32", "Win32-specific test") def test_execve_with_empty_path(self): # bpo-32890: Check GetLastError() misuse diff --git a/Misc/NEWS.d/next/Library/2025-12-31-17-38-33.gh-issue-143308.lY8UCR.rst b/Misc/NEWS.d/next/Library/2025-12-31-17-38-33.gh-issue-143308.lY8UCR.rst new file mode 100644 index 00000000000000..5db43b3d6d5630 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-12-31-17-38-33.gh-issue-143308.lY8UCR.rst @@ -0,0 +1,3 @@ +:mod:`pickle`: fix use-after-free crashes when a :class:`~pickle.PickleBuffer` +is concurrently mutated by a custom buffer callback during pickling. +Patch by Bénédikt Tran and Aaron Wieczorek. diff --git a/Misc/NEWS.d/next/Library/2025-12-31-20-43-02.gh-issue-143309.cdFxdH.rst b/Misc/NEWS.d/next/Library/2025-12-31-20-43-02.gh-issue-143309.cdFxdH.rst new file mode 100644 index 00000000000000..5f30ed340bf0eb --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-12-31-20-43-02.gh-issue-143309.cdFxdH.rst @@ -0,0 +1,3 @@ +Fix a crash in :func:`os.execve` on non-Windows platforms when +given a custom environment mapping which is then mutated during +parsing. Patch by Bénédikt Tran. diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 608598eb5a536c..d4a610e7909b5e 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -2636,53 +2636,61 @@ save_picklebuffer(PickleState *st, PicklerObject *self, PyObject *obj) "PickleBuffer can only be pickled with protocol >= 5"); return -1; } - const Py_buffer* view = PyPickleBuffer_GetBuffer(obj); - if (view == NULL) { + Py_buffer view; + if (PyObject_GetBuffer(obj, &view, PyBUF_FULL_RO) != 0) { return -1; } - if (view->suboffsets != NULL || !PyBuffer_IsContiguous(view, 'A')) { + if (view.suboffsets != NULL || !PyBuffer_IsContiguous(&view, 'A')) { PyErr_SetString(st->PicklingError, "PickleBuffer can not be pickled when " "pointing to a non-contiguous buffer"); - return -1; + goto error; } + + int rc = 0; int in_band = 1; if (self->buffer_callback != NULL) { PyObject *ret = PyObject_CallOneArg(self->buffer_callback, obj); if (ret == NULL) { - return -1; + goto error; } in_band = PyObject_IsTrue(ret); Py_DECREF(ret); if (in_band == -1) { - return -1; + goto error; } } if (in_band) { /* Write data in-band */ - if (view->readonly) { - return _save_bytes_data(st, self, obj, (const char *)view->buf, - view->len); + if (view.readonly) { + rc = _save_bytes_data(st, self, obj, (const char *)view.buf, + view.len); } else { - return _save_bytearray_data(st, self, obj, (const char *)view->buf, - view->len); + rc = _save_bytearray_data(st, self, obj, (const char *)view.buf, + view.len); } } else { /* Write data out-of-band */ const char next_buffer_op = NEXT_BUFFER; if (_Pickler_Write(self, &next_buffer_op, 1) < 0) { - return -1; + goto error; } - if (view->readonly) { + if (view.readonly) { const char readonly_buffer_op = READONLY_BUFFER; if (_Pickler_Write(self, &readonly_buffer_op, 1) < 0) { - return -1; + goto error; } } } - return 0; + + PyBuffer_Release(&view); + return rc; + +error: + PyBuffer_Release(&view); + return -1; } /* A copy of PyUnicode_AsRawUnicodeEscapeString() that also translates diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index e0276ce9e3906f..b26785abc3cd27 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -7291,8 +7291,8 @@ static EXECV_CHAR** parse_envlist(PyObject* env, Py_ssize_t *envc_ptr) { Py_ssize_t i, pos, envc; - PyObject *keys=NULL, *vals=NULL; - PyObject *key2, *val2, *keyval; + PyObject *keys = NULL, *vals = NULL; + PyObject *key = NULL, *val = NULL, *key2 = NULL, *val2 = NULL; EXECV_CHAR **envlist; i = PyMapping_Size(env); @@ -7317,20 +7317,22 @@ parse_envlist(PyObject* env, Py_ssize_t *envc_ptr) } for (pos = 0; pos < i; pos++) { - PyObject *key = PyList_GetItem(keys, pos); // Borrowed ref. + // The 'key' and 'val' must be strong references because of + // possible side-effects by PyUnicode_FS{Converter,Decoder}(). + key = PyList_GetItemRef(keys, pos); if (key == NULL) { goto error; } - PyObject *val = PyList_GetItem(vals, pos); // Borrowed ref. + val = PyList_GetItemRef(vals, pos); if (val == NULL) { goto error; } #if defined(HAVE_WEXECV) || defined(HAVE_WSPAWNV) - if (!PyUnicode_FSDecoder(key, &key2)) + if (!PyUnicode_FSDecoder(key, &key2)) { goto error; + } if (!PyUnicode_FSDecoder(val, &val2)) { - Py_DECREF(key2); goto error; } /* Search from index 1 because on Windows starting '=' is allowed for @@ -7339,39 +7341,38 @@ parse_envlist(PyObject* env, Py_ssize_t *envc_ptr) PyUnicode_FindChar(key2, '=', 1, PyUnicode_GET_LENGTH(key2), 1) != -1) { PyErr_SetString(PyExc_ValueError, "illegal environment variable name"); - Py_DECREF(key2); - Py_DECREF(val2); goto error; } - keyval = PyUnicode_FromFormat("%U=%U", key2, val2); + PyObject *keyval = PyUnicode_FromFormat("%U=%U", key2, val2); #else - if (!PyUnicode_FSConverter(key, &key2)) + if (!PyUnicode_FSConverter(key, &key2)) { goto error; + } if (!PyUnicode_FSConverter(val, &val2)) { - Py_DECREF(key2); goto error; } if (PyBytes_GET_SIZE(key2) == 0 || strchr(PyBytes_AS_STRING(key2) + 1, '=') != NULL) { PyErr_SetString(PyExc_ValueError, "illegal environment variable name"); - Py_DECREF(key2); - Py_DECREF(val2); goto error; } - keyval = PyBytes_FromFormat("%s=%s", PyBytes_AS_STRING(key2), - PyBytes_AS_STRING(val2)); + PyObject *keyval = PyBytes_FromFormat("%s=%s", PyBytes_AS_STRING(key2), + PyBytes_AS_STRING(val2)); #endif - Py_DECREF(key2); - Py_DECREF(val2); - if (!keyval) + if (!keyval) { goto error; + } if (!fsconvert_strdup(keyval, &envlist[envc++])) { Py_DECREF(keyval); goto error; } + Py_CLEAR(key); + Py_CLEAR(val); + Py_CLEAR(key2); + Py_CLEAR(val2); Py_DECREF(keyval); } Py_DECREF(vals); @@ -7382,6 +7383,10 @@ parse_envlist(PyObject* env, Py_ssize_t *envc_ptr) return envlist; error: + Py_XDECREF(key); + Py_XDECREF(val); + Py_XDECREF(key2); + Py_XDECREF(val2); Py_XDECREF(keys); Py_XDECREF(vals); free_string_array(envlist, envc);