From: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> Date: Wed, 8 Oct 2025 15:22:44 +0000 (+0200) Subject: gh-139748: fix leaks in AC error paths when using unicode FS-based converters (#139765) X-Git-Tag: v3.15.0a1~82 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b04a57deef66ce08233be57d1ab5873388df2cea;p=thirdparty%2FPython%2Fcpython.git gh-139748: fix leaks in AC error paths when using unicode FS-based converters (#139765) --- diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index d54dd546ea36..e0dbb062eb03 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -2980,6 +2980,8 @@ class ClinicExternalTest(TestCase): "uint64", "uint8", "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 277a2a187543..1660dabe6815 100644 --- a/Lib/test/test_compile.py +++ b/Lib/test/test_compile.py @@ -651,6 +651,21 @@ class TestSpecifics(unittest.TestCase): 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 26c75fcbc2bd..943e63fc13c9 100644 --- a/Lib/test/test_symtable.py +++ b/Lib/test/test_symtable.py @@ -579,6 +579,13 @@ class SymtableTest(unittest.TestCase): self.assertEqual(sorted(st.get_identifiers()), [".0", "y"]) self.assertEqual(st.get_children(), []) + 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): 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 000000000000..3b190f3f6df6 --- /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 e1bdc4033ba1..1fa44ef1de4e 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -1858,14 +1858,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; @@ -1895,7 +1895,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 0e8e4a7c8a73..d1fb024903e1 100644 --- a/Modules/clinic/_ssl.c.h +++ b/Modules/clinic/_ssl.c.h @@ -48,7 +48,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; @@ -56,6 +56,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; } @@ -3322,4 +3325,4 @@ exit: #ifndef _SSL_ENUM_CRLS_METHODDEF #define _SSL_ENUM_CRLS_METHODDEF #endif /* !defined(_SSL_ENUM_CRLS_METHODDEF) */ -/*[clinic end generated code: output=5a630a1e83927d47 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=3b6c9cbfc4660ecb input=a9049054013a1b77]*/ diff --git a/Modules/clinic/socketmodule.c.h b/Modules/clinic/socketmodule.c.h index 0cedab597db7..e0cc1c50dcba 100644 --- a/Modules/clinic/socketmodule.c.h +++ b/Modules/clinic/socketmodule.c.h @@ -479,7 +479,7 @@ static PyObject * _socket_if_nametoindex(PyObject *module, PyObject *arg) { PyObject *return_value = NULL; - PyObject *oname; + PyObject *oname = NULL; if (!PyUnicode_FSConverter(arg, &oname)) { goto exit; @@ -487,6 +487,9 @@ _socket_if_nametoindex(PyObject *module, PyObject *arg) return_value = _socket_if_nametoindex_impl(module, oname); exit: + /* Cleanup for oname */ + Py_XDECREF(oname); + return return_value; } @@ -538,4 +541,4 @@ exit: #ifndef _SOCKET_IF_INDEXTONAME_METHODDEF #define _SOCKET_IF_INDEXTONAME_METHODDEF #endif /* !defined(_SOCKET_IF_INDEXTONAME_METHODDEF) */ -/*[clinic end generated code: output=0376c46b76ae2bce input=a9049054013a1b77]*/ +/*[clinic end generated code: output=36051ebf6ad1e6f8 input=a9049054013a1b77]*/ diff --git a/Modules/clinic/symtablemodule.c.h b/Modules/clinic/symtablemodule.c.h index 2ecd3afc00d2..bd55d77c5409 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 f50167c223e2..4189d3008560 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3137,17 +3137,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 "' @@ -3211,7 +3200,7 @@ class confname_converter(CConverter): """, argname=argname, converter=self.converter, table=self.table) [python start generated code]*/ -/*[python end generated code: output=da39a3ee5e6b4b0d input=8189d5ae78244626]*/ +/*[python end generated code: output=da39a3ee5e6b4b0d input=d2759f2332cd39b3]*/ /*[clinic input] @@ -6135,14 +6124,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); @@ -9328,7 +9317,7 @@ error: /*[clinic input] os.initgroups - username as oname: FSConverter + username as oname: unicode_fs_encoded gid: int / @@ -9341,12 +9330,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 / @@ -9359,7 +9348,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); @@ -13115,8 +13104,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. @@ -13124,7 +13113,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); @@ -13167,7 +13156,7 @@ os_unsetenv_impl(PyObject *module, PyObject *name) #else /*[clinic input] os.unsetenv - name: FSConverter + name: unicode_fs_encoded / Delete an environment variable. @@ -13175,7 +13164,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; @@ -15229,14 +15218,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 fa153efd8b72..dd4b68929774 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -7278,7 +7278,7 @@ Returns a list of network interface information (index, name) tuples."); /*[clinic input] _socket.if_nametoindex - oname: object(converter="PyUnicode_FSConverter") + oname: unicode_fs_encoded / Returns the interface index corresponding to the interface name if_name. @@ -7286,7 +7286,7 @@ Returns the interface index corresponding to the interface name if_name. static PyObject * _socket_if_nametoindex_impl(PyObject *module, PyObject *oname) -/*[clinic end generated code: output=289a411614f30244 input=01e0f1205307fb77]*/ +/*[clinic end generated code: output=289a411614f30244 input=6125dc20683560cf]*/ { #ifdef MS_WINDOWS NET_IFINDEX index; @@ -7295,7 +7295,6 @@ _socket_if_nametoindex_impl(PyObject *module, 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 d0d5223e5ace..d353f406831e 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 b7e08ae54da5..2551c2c961b6 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -745,7 +745,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 @@ -771,7 +771,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; @@ -889,7 +889,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 60e20a46af5b..adb82f45c25b 100644 --- a/Python/clinic/bltinmodule.c.h +++ b/Python/clinic/bltinmodule.c.h @@ -296,7 +296,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; @@ -367,6 +367,9 @@ skip_optional_kwonly: 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; } @@ -1274,4 +1277,4 @@ builtin_issubclass(PyObject *module, PyObject *const *args, Py_ssize_t nargs) exit: return return_value; } -/*[clinic end generated code: output=c0b72519622c849e input=a9049054013a1b77]*/ +/*[clinic end generated code: output=7eada753dc2e046f input=a9049054013a1b77]*/ diff --git a/Tools/clinic/libclinic/converters.py b/Tools/clinic/libclinic/converters.py index 3154299e31b4..201125b9165e 100644 --- a/Tools/clinic/libclinic/converters.py +++ b/Tools/clinic/libclinic/converters.py @@ -916,6 +916,26 @@ class unicode_converter(CConverter): 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})