From 38b2a4ff3190eb4f2db3ba278d2c7d24b55200c9 Mon Sep 17 00:00:00 2001 From: tonghuaroot <23011166+tonghuaroot@users.noreply.github.com> Date: Fri, 12 Jun 2026 22:46:59 +0800 Subject: [PATCH] gh-151403: Fix use-after-free when an argv item's __fspath__ mutates args --- Lib/test/test_subprocess.py | 39 +++++++++++++++++++ ...-06-12-22-46-31.gh-issue-151403.DalZWh.rst | 3 ++ Modules/_posixsubprocess.c | 8 +++- 3 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Library/2026-06-12-22-46-31.gh-issue-151403.DalZWh.rst diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index d41cb1294a3dafe..2786644b1e98dba 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -3476,6 +3476,45 @@ def test_fork_exec(self): if not gc_enabled: gc.disable() + @support.cpython_only + def test_fork_exec_args_concurrent_mutation(self): + # An argv item's __fspath__() can run Python that drops the args + # sequence's last reference to that item while fork_exec() is still + # converting it. See: https://github.com/python/cpython/issues/151403 + import _posixsubprocess + + def fork_exec(args): + return _posixsubprocess.fork_exec( + args, [b"false"], + True, (), None, [b"env"], + -1, -1, -1, -1, + 1, 2, 3, 4, + True, True, 0, + None, None, None, -1, + None) + + # __fspath__() returns a non-str/bytes, forcing the error path that + # reports the offending item's type. Clearing the list there drops + # the item's last reference: only fork_exec() keeping its own + # reference avoids a use-after-free. + class Evil: + def __fspath__(self): + args.clear() + return 12345 + args = [Evil()] + with self.assertRaises(TypeError): + fork_exec(args) + + # Changing the list length is reported as a RuntimeError before the + # next item is converted. + class Shrink: + def __fspath__(self): + args.clear() + return "/nonexistent" + args = [Shrink(), "/second"] + with self.assertRaises(RuntimeError): + fork_exec(args) + @support.cpython_only def test_fork_exec_sorted_fd_sanity_check(self): # Issue #23564: sanity check the fork_exec() fds_to_keep sanity check. diff --git a/Misc/NEWS.d/next/Library/2026-06-12-22-46-31.gh-issue-151403.DalZWh.rst b/Misc/NEWS.d/next/Library/2026-06-12-22-46-31.gh-issue-151403.DalZWh.rst new file mode 100644 index 000000000000000..4b48f90bb207120 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-06-12-22-46-31.gh-issue-151403.DalZWh.rst @@ -0,0 +1,3 @@ +Fixed a crash in :class:`subprocess.Popen` (and ``_posixsubprocess.fork_exec``) +when an ``argv`` item's :meth:`~object.__fspath__` concurrently mutates the +``args`` sequence being converted. diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index ddc27c4a5b7356a..2aa3923f68e66ad 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -1090,8 +1090,14 @@ subprocess_fork_exec_impl(PyObject *module, PyObject *process_args, goto cleanup; } borrowed_arg = PySequence_Fast_GET_ITEM(fast_args, arg_num); - if (PyUnicode_FSConverter(borrowed_arg, &converted_arg) == 0) + /* borrowed_arg is only borrowed; its __fspath__() may run Python + that drops fast_args' last reference to it. */ + Py_INCREF(borrowed_arg); + if (PyUnicode_FSConverter(borrowed_arg, &converted_arg) == 0) { + Py_DECREF(borrowed_arg); goto cleanup; + } + Py_DECREF(borrowed_arg); PyTuple_SET_ITEM(converted_args, arg_num, converted_arg); }