From 786b335b0a2d1b594af41e7092e582aa221018ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 8 Oct 2025 17:22:44 +0200 Subject: [PATCH 1/2] [3.13] gh-139748: fix leaks in AC error paths when using unicode FS-based converters (GH-139765) (cherry picked from commit b04a57deef66ce08233be57d1ab5873388df2cea) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> --- Lib/test/test_clinic.py | 2 + Lib/test/test_compile.py | 15 +++++++ Lib/test/test_symtable.py | 28 +++++++++++++ ...-10-08-13-52-00.gh-issue-139748.jq0yFJ.rst | 2 + Modules/_ssl.c | 5 +-- Modules/clinic/_ssl.c.h | 7 +++- Modules/clinic/symtablemodule.c.h | 7 +++- Modules/posixmodule.c | 39 +++++++------------ Modules/socketmodule.c | 1 - Modules/symtablemodule.c | 6 +-- Python/bltinmodule.c | 5 +-- Python/clinic/bltinmodule.c.h | 7 +++- Tools/clinic/libclinic/converters.py | 20 ++++++++++ 13 files changed, 102 insertions(+), 42 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2025-10-08-13-52-00.gh-issue-139748.jq0yFJ.rst diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index c3c566d4b332c4..737644ff1be5fe 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -2827,6 +2827,8 @@ def test_cli_converters(self): "slice_index", "str", "unicode", + "unicode_fs_decoded", + "unicode_fs_encoded", "unsigned_char", "unsigned_int", "unsigned_long", diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py index e4be26322930e0..eda2c0c40494ad 100644 --- a/Lib/test/test_compile.py +++ b/Lib/test/test_compile.py @@ -643,6 +643,21 @@ def test_compile_filename(self): compile('pass', filename, 'exec') self.assertRaises(TypeError, compile, 'pass', list(b'file.py'), 'exec') + def test_compile_filename_refleak(self): + # Regression tests for reference leak in PyUnicode_FSDecoder. + # See https://github.com/python/cpython/issues/139748. + mortal_str = 'this is a mortal string' + # check error path when 'mode' AC conversion failed + self.assertRaises(TypeError, compile, b'', mortal_str, mode=1234) + # check error path when 'optimize' AC conversion failed + self.assertRaises(OverflowError, compile, b'', mortal_str, + 'exec', optimize=1 << 1000) + # check error path when 'dont_inherit' AC conversion failed + class EvilBool: + def __bool__(self): raise ValueError + self.assertRaises(ValueError, compile, b'', mortal_str, + 'exec', dont_inherit=EvilBool()) + @support.cpython_only def test_same_filename_used(self): s = """def f(): pass\ndef g(): pass""" diff --git a/Lib/test/test_symtable.py b/Lib/test/test_symtable.py index c1b7030d2d250c..9789176020124f 100644 --- a/Lib/test/test_symtable.py +++ b/Lib/test/test_symtable.py @@ -493,6 +493,34 @@ def test_symtable_entry_repr(self): expected = f"" self.assertEqual(repr(self.top._table), expected) + def test__symtable_refleak(self): + # Regression test for reference leak in PyUnicode_FSDecoder. + # See https://github.com/python/cpython/issues/139748. + mortal_str = 'this is a mortal string' + # check error path when 'compile_type' AC conversion failed + self.assertRaises(TypeError, symtable.symtable, '', mortal_str, 1) + + +class ComprehensionTests(unittest.TestCase): + def get_identifiers_recursive(self, st, res): + res.extend(st.get_identifiers()) + for ch in st.get_children(): + self.get_identifiers_recursive(ch, res) + + def test_loopvar_in_only_one_scope(self): + # ensure that the loop variable appears only once in the symtable + comps = [ + "[x for x in [1]]", + "{x for x in [1]}", + "{x:x*x for x in [1]}", + ] + for comp in comps: + with self.subTest(comp=comp): + st = symtable.symtable(comp, "?", "exec") + ids = [] + self.get_identifiers_recursive(st, ids) + self.assertEqual(len([x for x in ids if x == 'x']), 1) + class CommandLineTest(unittest.TestCase): maxDiff = None diff --git a/Misc/NEWS.d/next/Core and Builtins/2025-10-08-13-52-00.gh-issue-139748.jq0yFJ.rst b/Misc/NEWS.d/next/Core and Builtins/2025-10-08-13-52-00.gh-issue-139748.jq0yFJ.rst new file mode 100644 index 00000000000000..3b190f3f6df6e3 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2025-10-08-13-52-00.gh-issue-139748.jq0yFJ.rst @@ -0,0 +1,2 @@ +Fix reference leaks in error branches of functions accepting path strings or +bytes such as :func:`compile` and :func:`os.system`. Patch by Bénédikt Tran. diff --git a/Modules/_ssl.c b/Modules/_ssl.c index fd8c16b92547c8..970350a187b298 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -1782,14 +1782,14 @@ _certificate_to_der(_sslmodulestate *state, X509 *certificate) /*[clinic input] _ssl._test_decode_cert - path: object(converter="PyUnicode_FSConverter") + path: unicode_fs_encoded / [clinic start generated code]*/ static PyObject * _ssl__test_decode_cert_impl(PyObject *module, PyObject *path) -/*[clinic end generated code: output=96becb9abb23c091 input=cdeaaf02d4346628]*/ +/*[clinic end generated code: output=96becb9abb23c091 input=cb4988d5e651a4f8]*/ { PyObject *retval = NULL; X509 *x=NULL; @@ -1819,7 +1819,6 @@ _ssl__test_decode_cert_impl(PyObject *module, PyObject *path) X509_free(x); fail0: - Py_DECREF(path); if (cert != NULL) BIO_free(cert); return retval; } diff --git a/Modules/clinic/_ssl.c.h b/Modules/clinic/_ssl.c.h index 7c1ea6ad1798eb..628bd877fe1b81 100644 --- a/Modules/clinic/_ssl.c.h +++ b/Modules/clinic/_ssl.c.h @@ -47,7 +47,7 @@ static PyObject * _ssl__test_decode_cert(PyObject *module, PyObject *arg) { PyObject *return_value = NULL; - PyObject *path; + PyObject *path = NULL; if (!PyUnicode_FSConverter(arg, &path)) { goto exit; @@ -55,6 +55,9 @@ _ssl__test_decode_cert(PyObject *module, PyObject *arg) return_value = _ssl__test_decode_cert_impl(module, path); exit: + /* Cleanup for path */ + Py_XDECREF(path); + return return_value; } @@ -2867,4 +2870,4 @@ _ssl_enum_crls(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObje #ifndef _SSL_ENUM_CRLS_METHODDEF #define _SSL_ENUM_CRLS_METHODDEF #endif /* !defined(_SSL_ENUM_CRLS_METHODDEF) */ -/*[clinic end generated code: output=8c4a1e44702afeb7 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=6849453c6b91db74 input=a9049054013a1b77]*/ diff --git a/Modules/clinic/symtablemodule.c.h b/Modules/clinic/symtablemodule.c.h index 2ecd3afc00d2be..bd55d77c5409e9 100644 --- a/Modules/clinic/symtablemodule.c.h +++ b/Modules/clinic/symtablemodule.c.h @@ -22,7 +22,7 @@ _symtable_symtable(PyObject *module, PyObject *const *args, Py_ssize_t nargs) { PyObject *return_value = NULL; PyObject *source; - PyObject *filename; + PyObject *filename = NULL; const char *startstr; if (!_PyArg_CheckPositional("symtable", nargs, 3, 3)) { @@ -48,6 +48,9 @@ _symtable_symtable(PyObject *module, PyObject *const *args, Py_ssize_t nargs) return_value = _symtable_symtable_impl(module, source, filename, startstr); exit: + /* Cleanup for filename */ + Py_XDECREF(filename); + return return_value; } -/*[clinic end generated code: output=931964a76a72f850 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=7a8545d9a1efe837 input=a9049054013a1b77]*/ diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index b7d128b81304b0..9c1564093b96bc 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3044,17 +3044,6 @@ class dev_t_return_converter(unsigned_long_return_converter): conversion_fn = '_PyLong_FromDev' unsigned_cast = '(dev_t)' -class FSConverter_converter(CConverter): - type = 'PyObject *' - converter = 'PyUnicode_FSConverter' - def converter_init(self): - if self.default is not unspecified: - fail("FSConverter_converter does not support default values") - self.c_default = 'NULL' - - def cleanup(self): - return "Py_XDECREF(" + self.name + ");\n" - class pid_t_converter(CConverter): type = 'pid_t' format_unit = '" _Py_PARSE_PID "' @@ -3093,7 +3082,7 @@ class confname_converter(CConverter): """, argname=argname, converter=self.converter, table=self.table) [python start generated code]*/ -/*[python end generated code: output=da39a3ee5e6b4b0d input=a6199b1618d73f53]*/ +/*[python end generated code: output=da39a3ee5e6b4b0d input=1a078a0f42a586c1]*/ /*[clinic input] @@ -6013,14 +6002,14 @@ os_system_impl(PyObject *module, const wchar_t *command) /*[clinic input] os.system -> long - command: FSConverter + command: unicode_fs_encoded Execute the command in a subshell. [clinic start generated code]*/ static long os_system_impl(PyObject *module, PyObject *command) -/*[clinic end generated code: output=290fc437dd4f33a0 input=86a58554ba6094af]*/ +/*[clinic end generated code: output=290fc437dd4f33a0 input=47c6f24b6dc92881]*/ { long result; const char *bytes = PyBytes_AsString(command); @@ -9200,7 +9189,7 @@ os_getgroups_impl(PyObject *module) /*[clinic input] os.initgroups - username as oname: FSConverter + username as oname: unicode_fs_encoded gid: int / @@ -9213,12 +9202,12 @@ group id. static PyObject * os_initgroups_impl(PyObject *module, PyObject *oname, int gid) -/*[clinic end generated code: output=7f074d30a425fd3a input=df3d54331b0af204]*/ +/*[clinic end generated code: output=7f074d30a425fd3a input=984e60c7fed88cb4]*/ #else /*[clinic input] os.initgroups - username as oname: FSConverter + username as oname: unicode_fs_encoded gid: gid_t / @@ -9231,7 +9220,7 @@ group id. static PyObject * os_initgroups_impl(PyObject *module, PyObject *oname, gid_t gid) -/*[clinic end generated code: output=59341244521a9e3f input=0cb91bdc59a4c564]*/ +/*[clinic end generated code: output=59341244521a9e3f input=17d8fbe2dea42ca4]*/ #endif { const char *username = PyBytes_AS_STRING(oname); @@ -12956,8 +12945,8 @@ os_putenv_impl(PyObject *module, PyObject *name, PyObject *value) /*[clinic input] os.putenv - name: FSConverter - value: FSConverter + name: unicode_fs_encoded + value: unicode_fs_encoded / Change or add an environment variable. @@ -12965,7 +12954,7 @@ Change or add an environment variable. static PyObject * os_putenv_impl(PyObject *module, PyObject *name, PyObject *value) -/*[clinic end generated code: output=d29a567d6b2327d2 input=a97bc6152f688d31]*/ +/*[clinic end generated code: output=d29a567d6b2327d2 input=84fcd30f873c8c45]*/ { const char *name_string = PyBytes_AS_STRING(name); const char *value_string = PyBytes_AS_STRING(value); @@ -13008,7 +12997,7 @@ os_unsetenv_impl(PyObject *module, PyObject *name) #else /*[clinic input] os.unsetenv - name: FSConverter + name: unicode_fs_encoded / Delete an environment variable. @@ -13016,7 +13005,7 @@ Delete an environment variable. static PyObject * os_unsetenv_impl(PyObject *module, PyObject *name) -/*[clinic end generated code: output=54c4137ab1834f02 input=2bb5288a599c7107]*/ +/*[clinic end generated code: output=54c4137ab1834f02 input=78ff12e505ade80a]*/ { if (PySys_Audit("os.unsetenv", "(O)", name) < 0) { return NULL; @@ -15074,14 +15063,14 @@ os_urandom_impl(PyObject *module, Py_ssize_t size) /*[clinic input] os.memfd_create - name: FSConverter + name: unicode_fs_encoded flags: unsigned_int(bitwise=True, c_default="MFD_CLOEXEC") = MFD_CLOEXEC [clinic start generated code]*/ static PyObject * os_memfd_create_impl(PyObject *module, PyObject *name, unsigned int flags) -/*[clinic end generated code: output=6681ede983bdb9a6 input=a42cfc199bcd56e9]*/ +/*[clinic end generated code: output=6681ede983bdb9a6 input=cd0eb092cfac474b]*/ { int fd; const char *bytes = PyBytes_AS_STRING(name); diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 9a7c977bc5698e..a6910df6fba64e 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -7115,7 +7115,6 @@ _socket_socket_if_nametoindex_impl(PySocketSockObject *self, PyObject *oname) #endif index = if_nametoindex(PyBytes_AS_STRING(oname)); - Py_DECREF(oname); if (index == 0) { /* if_nametoindex() doesn't set errno */ PyErr_SetString(PyExc_OSError, "no interface with this name"); diff --git a/Modules/symtablemodule.c b/Modules/symtablemodule.c index 00e650a9adaaf7..562be6d3eb6a63 100644 --- a/Modules/symtablemodule.c +++ b/Modules/symtablemodule.c @@ -13,7 +13,7 @@ module _symtable _symtable.symtable source: object - filename: object(converter='PyUnicode_FSDecoder') + filename: unicode_fs_decoded startstr: str / @@ -23,7 +23,7 @@ Return symbol and scope dictionaries used internally by compiler. static PyObject * _symtable_symtable_impl(PyObject *module, PyObject *source, PyObject *filename, const char *startstr) -/*[clinic end generated code: output=59eb0d5fc7285ac4 input=9dd8a50c0c36a4d7]*/ +/*[clinic end generated code: output=59eb0d5fc7285ac4 input=436ffff90d02e4f6]*/ { struct symtable *st; PyObject *t; @@ -47,12 +47,10 @@ _symtable_symtable_impl(PyObject *module, PyObject *source, else { PyErr_SetString(PyExc_ValueError, "symtable() arg 3 must be 'exec' or 'eval' or 'single'"); - Py_DECREF(filename); Py_XDECREF(source_copy); return NULL; } st = _Py_SymtableStringObjectFlags(str, filename, start, &cf); - Py_DECREF(filename); Py_XDECREF(source_copy); if (st == NULL) { return NULL; diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index eaadf83667262e..5ded0bba13b16d 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -733,7 +733,7 @@ builtin_chr(PyObject *module, PyObject *i) compile as builtin_compile source: object - filename: object(converter="PyUnicode_FSDecoder") + filename: unicode_fs_decoded mode: str flags: int = 0 dont_inherit: bool = False @@ -759,7 +759,7 @@ static PyObject * builtin_compile_impl(PyObject *module, PyObject *source, PyObject *filename, const char *mode, int flags, int dont_inherit, int optimize, int feature_version) -/*[clinic end generated code: output=b0c09c84f116d3d7 input=cc78e20e7c7682ba]*/ +/*[clinic end generated code: output=b0c09c84f116d3d7 input=8f0069edbdac381b]*/ { PyObject *source_copy; const char *str; @@ -885,7 +885,6 @@ builtin_compile_impl(PyObject *module, PyObject *source, PyObject *filename, error: result = NULL; finally: - Py_DECREF(filename); return result; } diff --git a/Python/clinic/bltinmodule.c.h b/Python/clinic/bltinmodule.c.h index 2abdc6f3ec2ff6..370cf3c929b18d 100644 --- a/Python/clinic/bltinmodule.c.h +++ b/Python/clinic/bltinmodule.c.h @@ -291,7 +291,7 @@ builtin_compile(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObj PyObject *argsbuf[7]; Py_ssize_t noptargs = nargs + (kwnames ? PyTuple_GET_SIZE(kwnames) : 0) - 3; PyObject *source; - PyObject *filename; + PyObject *filename = NULL; const char *mode; int flags = 0; int dont_inherit = 0; @@ -361,6 +361,9 @@ builtin_compile(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObj return_value = builtin_compile_impl(module, source, filename, mode, flags, dont_inherit, optimize, feature_version); exit: + /* Cleanup for filename */ + Py_XDECREF(filename); + return return_value; } @@ -1234,4 +1237,4 @@ builtin_issubclass(PyObject *module, PyObject *const *args, Py_ssize_t nargs) exit: return return_value; } -/*[clinic end generated code: output=f686ad843368963c input=a9049054013a1b77]*/ +/*[clinic end generated code: output=39173c70790d9cd0 input=a9049054013a1b77]*/ diff --git a/Tools/clinic/libclinic/converters.py b/Tools/clinic/libclinic/converters.py index 978bd94fd1304d..238237d7b2ec83 100644 --- a/Tools/clinic/libclinic/converters.py +++ b/Tools/clinic/libclinic/converters.py @@ -937,6 +937,26 @@ def parse_arg(self, argname: str, displayname: str, *, limited_capi: bool) -> st return super().parse_arg(argname, displayname, limited_capi=limited_capi) +class _unicode_fs_converter_base(CConverter): + type = 'PyObject *' + + def converter_init(self) -> None: + if self.default is not unspecified: + fail(f"{self.__class__.__name__} does not support default values") + self.c_default = 'NULL' + + def cleanup(self) -> str: + return f"Py_XDECREF({self.parser_name});" + + +class unicode_fs_encoded_converter(_unicode_fs_converter_base): + converter = 'PyUnicode_FSConverter' + + +class unicode_fs_decoded_converter(_unicode_fs_converter_base): + converter = 'PyUnicode_FSDecoder' + + @add_legacy_c_converter('u') @add_legacy_c_converter('u#', zeroes=True) @add_legacy_c_converter('Z', accept={str, NoneType}) From bbe82faeca47438593900e9cce1908c283ac70c3 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Wed, 8 Oct 2025 21:12:35 +0530 Subject: [PATCH 2/2] fix bad merge --- Lib/test/test_symtable.py | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/Lib/test/test_symtable.py b/Lib/test/test_symtable.py index 9789176020124f..c07d3113dfe9f0 100644 --- a/Lib/test/test_symtable.py +++ b/Lib/test/test_symtable.py @@ -501,27 +501,6 @@ def test__symtable_refleak(self): self.assertRaises(TypeError, symtable.symtable, '', mortal_str, 1) -class ComprehensionTests(unittest.TestCase): - def get_identifiers_recursive(self, st, res): - res.extend(st.get_identifiers()) - for ch in st.get_children(): - self.get_identifiers_recursive(ch, res) - - def test_loopvar_in_only_one_scope(self): - # ensure that the loop variable appears only once in the symtable - comps = [ - "[x for x in [1]]", - "{x for x in [1]}", - "{x:x*x for x in [1]}", - ] - for comp in comps: - with self.subTest(comp=comp): - st = symtable.symtable(comp, "?", "exec") - ids = [] - self.get_identifiers_recursive(st, ids) - self.assertEqual(len([x for x in ids if x == 'x']), 1) - - class CommandLineTest(unittest.TestCase): maxDiff = None