Skip to content

Commit bcdd98d

Browse files
committed
pythongh-143309: [3.13] fix test_execve_env_concurrent_mutation_with_fspath_posix buildbot failure (python#143415)
1 parent 2b8ac2c commit bcdd98d

File tree

3 files changed

+65
-18
lines changed

3 files changed

+65
-18
lines changed

Lib/test/test_os.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2255,6 +2255,45 @@ def test_execve_invalid_env(self):
22552255
with self.assertRaises(ValueError):
22562256
os.execve(args[0], args, newenv)
22572257

2258+
# See https://github.com/python/cpython/issues/137934 and the other
2259+
# related issues for the reason why we cannot test this on Windows.
2260+
@unittest.skipIf(os.name == "nt", "POSIX-specific test")
2261+
@unittest.skipUnless(unix_shell and os.path.exists(unix_shell),
2262+
"requires a shell")
2263+
def test_execve_env_concurrent_mutation_with_fspath_posix(self):
2264+
# Prevent crash when mutating environment during parsing.
2265+
# Regression test for https://github.com/python/cpython/issues/143309.
2266+
2267+
message = "hello from execve"
2268+
code = """if 1:
2269+
import os, sys
2270+
2271+
class MyPath:
2272+
def __fspath__(self):
2273+
mutated.clear()
2274+
return b"pwn"
2275+
2276+
mutated = KEYS = VALUES = [MyPath()]
2277+
2278+
class MyEnv:
2279+
def __getitem__(self): raise RuntimeError("must not be called")
2280+
def __len__(self): return 1
2281+
def keys(self): return KEYS
2282+
def values(self): return VALUES
2283+
2284+
args = [{unix_shell!r}, '-c', 'echo \"{message!s}\"']
2285+
os.execve(args[0], args, MyEnv())
2286+
""".format(unix_shell=unix_shell, message=message)
2287+
2288+
# Make sure to forward "LD_*" variables so that assert_python_ok()
2289+
# can run correctly.
2290+
minimal = {k: v for k, v in os.environ.items() if k.startswith("LD_")}
2291+
with os_helper.EnvironmentVarGuard() as env:
2292+
env.clear()
2293+
env.update(minimal)
2294+
_, out, _ = assert_python_ok('-c', code, **env)
2295+
self.assertIn(bytes(message, "ascii"), out)
2296+
22582297
@unittest.skipUnless(sys.platform == "win32", "Win32-specific test")
22592298
def test_execve_with_empty_path(self):
22602299
# bpo-32890: Check GetLastError() misuse
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix a crash in :func:`os.execve` on non-Windows platforms when
2+
given a custom environment mapping which is then mutated during
3+
parsing. Patch by Bénédikt Tran.

Modules/posixmodule.c

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6740,8 +6740,8 @@ static EXECV_CHAR**
67406740
parse_envlist(PyObject* env, Py_ssize_t *envc_ptr)
67416741
{
67426742
Py_ssize_t i, pos, envc;
6743-
PyObject *keys=NULL, *vals=NULL;
6744-
PyObject *key2, *val2, *keyval;
6743+
PyObject *keys = NULL, *vals = NULL;
6744+
PyObject *key = NULL, *val = NULL, *key2 = NULL, *val2 = NULL;
67456745
EXECV_CHAR **envlist;
67466746

67476747
i = PyMapping_Size(env);
@@ -6766,20 +6766,22 @@ parse_envlist(PyObject* env, Py_ssize_t *envc_ptr)
67666766
}
67676767

67686768
for (pos = 0; pos < i; pos++) {
6769-
PyObject *key = PyList_GetItem(keys, pos); // Borrowed ref.
6769+
// The 'key' and 'val' must be strong references because of
6770+
// possible side-effects by PyUnicode_FS{Converter,Decoder}().
6771+
key = PyList_GetItemRef(keys, pos);
67706772
if (key == NULL) {
67716773
goto error;
67726774
}
6773-
PyObject *val = PyList_GetItem(vals, pos); // Borrowed ref.
6775+
val = PyList_GetItemRef(vals, pos);
67746776
if (val == NULL) {
67756777
goto error;
67766778
}
67776779

67786780
#if defined(HAVE_WEXECV) || defined(HAVE_WSPAWNV)
6779-
if (!PyUnicode_FSDecoder(key, &key2))
6781+
if (!PyUnicode_FSDecoder(key, &key2)) {
67806782
goto error;
6783+
}
67816784
if (!PyUnicode_FSDecoder(val, &val2)) {
6782-
Py_DECREF(key2);
67836785
goto error;
67846786
}
67856787
/* Search from index 1 because on Windows starting '=' is allowed for
@@ -6788,39 +6790,38 @@ parse_envlist(PyObject* env, Py_ssize_t *envc_ptr)
67886790
PyUnicode_FindChar(key2, '=', 1, PyUnicode_GET_LENGTH(key2), 1) != -1)
67896791
{
67906792
PyErr_SetString(PyExc_ValueError, "illegal environment variable name");
6791-
Py_DECREF(key2);
6792-
Py_DECREF(val2);
67936793
goto error;
67946794
}
6795-
keyval = PyUnicode_FromFormat("%U=%U", key2, val2);
6795+
PyObject *keyval = PyUnicode_FromFormat("%U=%U", key2, val2);
67966796
#else
6797-
if (!PyUnicode_FSConverter(key, &key2))
6797+
if (!PyUnicode_FSConverter(key, &key2)) {
67986798
goto error;
6799+
}
67996800
if (!PyUnicode_FSConverter(val, &val2)) {
6800-
Py_DECREF(key2);
68016801
goto error;
68026802
}
68036803
if (PyBytes_GET_SIZE(key2) == 0 ||
68046804
strchr(PyBytes_AS_STRING(key2) + 1, '=') != NULL)
68056805
{
68066806
PyErr_SetString(PyExc_ValueError, "illegal environment variable name");
6807-
Py_DECREF(key2);
6808-
Py_DECREF(val2);
68096807
goto error;
68106808
}
6811-
keyval = PyBytes_FromFormat("%s=%s", PyBytes_AS_STRING(key2),
6812-
PyBytes_AS_STRING(val2));
6809+
PyObject *keyval = PyBytes_FromFormat("%s=%s", PyBytes_AS_STRING(key2),
6810+
PyBytes_AS_STRING(val2));
68136811
#endif
6814-
Py_DECREF(key2);
6815-
Py_DECREF(val2);
6816-
if (!keyval)
6812+
if (!keyval) {
68176813
goto error;
6814+
}
68186815

68196816
if (!fsconvert_strdup(keyval, &envlist[envc++])) {
68206817
Py_DECREF(keyval);
68216818
goto error;
68226819
}
68236820

6821+
Py_CLEAR(key);
6822+
Py_CLEAR(val);
6823+
Py_CLEAR(key2);
6824+
Py_CLEAR(val2);
68246825
Py_DECREF(keyval);
68256826
}
68266827
Py_DECREF(vals);
@@ -6831,6 +6832,10 @@ parse_envlist(PyObject* env, Py_ssize_t *envc_ptr)
68316832
return envlist;
68326833

68336834
error:
6835+
Py_XDECREF(key);
6836+
Py_XDECREF(val);
6837+
Py_XDECREF(key2);
6838+
Py_XDECREF(val2);
68346839
Py_XDECREF(keys);
68356840
Py_XDECREF(vals);
68366841
free_string_array(envlist, envc);

0 commit comments

Comments
 (0)