Issue #26168: Fixed possible refleaks in failing Py_BuildValue() with the "N"

format unit.
2024-09-15 23:46:26 +00:00 · 2016-05-20 22:31:14 +03:00 · 2016-05-20 22:31:14 +03:00 · 13e602ea0f
parent 3dc5129d4f
commit 13e602ea0f
4 changed files with 173 additions and 55 deletions
--- a/Lib/test/test_capi.py
+++ b/Lib/test/test_capi.py
@ -236,6 +236,9 @@ def test_return_result_with_error(self):
                             'return_result_with_error.* '
                             'returned a result with an error set')

+    def test_buildvalue_N(self):
+        _testcapi.test_buildvalue_N()
+

@unittest.skipUnless(threading, 'Threading required for this test.')
 class TestPendingCalls(unittest.TestCase):
--- a/Misc/NEWS
+++ b/Misc/NEWS
@ -10,6 +10,9 @@ Release date: tba
 Core and Builtins
 -----------------

+- Issue #26168: Fixed possible refleaks in failing Py_BuildValue() with the "N"
+  format unit.
+
 - Issue #26991: Fix possible refleak when creating a function with annotations.

 - Issue #27039: Fixed bytearray.remove() for values greater than 127.  Patch by
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@ -872,6 +872,100 @@ test_L_code(PyObject *self)

 #endif  /* ifdef HAVE_LONG_LONG */

+static PyObject *
+return_none(void *unused)
+{
+    Py_RETURN_NONE;
+}
+
+static PyObject *
+raise_error(void *unused)
+{
+    PyErr_SetNone(PyExc_ValueError);
+    return NULL;
+}
+
+static int
+test_buildvalue_N_error(const char *fmt)
+{
+    PyObject *arg, *res;
+
+    arg = PyList_New(0);
+    if (arg == NULL) {
+        return -1;
+    }
+
+    Py_INCREF(arg);
+    res = Py_BuildValue(fmt, return_none, NULL, arg);
+    if (res == NULL) {
+        return -1;
+    }
+    Py_DECREF(res);
+    if (Py_REFCNT(arg) != 1) {
+        PyErr_Format(TestError, "test_buildvalue_N: "
+                     "arg was not decrefed in successful "
+                     "Py_BuildValue(\"%s\")", fmt);
+        return -1;
+    }
+
+    Py_INCREF(arg);
+    res = Py_BuildValue(fmt, raise_error, NULL, arg);
+    if (res != NULL || !PyErr_Occurred()) {
+        PyErr_Format(TestError, "test_buildvalue_N: "
+                     "Py_BuildValue(\"%s\") didn't complain", fmt);
+        return -1;
+    }
+    PyErr_Clear();
+    if (Py_REFCNT(arg) != 1) {
+        PyErr_Format(TestError, "test_buildvalue_N: "
+                     "arg was not decrefed in failed "
+                     "Py_BuildValue(\"%s\")", fmt);
+        return -1;
+    }
+    Py_DECREF(arg);
+    return 0;
+}
+
+static PyObject *
+test_buildvalue_N(PyObject *self, PyObject *noargs)
+{
+    PyObject *arg, *res;
+
+    arg = PyList_New(0);
+    if (arg == NULL) {
+        return NULL;
+    }
+    Py_INCREF(arg);
+    res = Py_BuildValue("N", arg);
+    if (res == NULL) {
+        return NULL;
+    }
+    if (res != arg) {
+        return raiseTestError("test_buildvalue_N",
+                              "Py_BuildValue(\"N\") returned wrong result");
+    }
+    if (Py_REFCNT(arg) != 2) {
+        return raiseTestError("test_buildvalue_N",
+                              "arg was not decrefed in Py_BuildValue(\"N\")");
+    }
+    Py_DECREF(res);
+    Py_DECREF(arg);
+
+    if (test_buildvalue_N_error("O&N") < 0)
+        return NULL;
+    if (test_buildvalue_N_error("(O&N)") < 0)
+        return NULL;
+    if (test_buildvalue_N_error("[O&N]") < 0)
+        return NULL;
+    if (test_buildvalue_N_error("{O&N}") < 0)
+        return NULL;
+    if (test_buildvalue_N_error("{()O&(())N}") < 0)
+        return NULL;
+
+    Py_RETURN_NONE;
+}
+
+
 static PyObject *
 get_args(PyObject *self, PyObject *args)
 {
@ -3728,6 +3822,7 @@ static PyMethodDef TestMethods[] = {
    {"test_pep3118_obsolete_write_locks", (PyCFunction)test_pep3118_obsolete_write_locks, METH_NOARGS},
 #endif
    {"getbuffer_with_null_view", getbuffer_with_null_view, METH_O},
+    {"test_buildvalue_N",       test_buildvalue_N,               METH_NOARGS},
    {"get_args", get_args, METH_VARARGS},
    {"get_kwargs", (PyCFunction)get_kwargs, METH_VARARGS|METH_KEYWORDS},
    {"getargs_tuple",           getargs_tuple,                   METH_VARARGS},
--- a/Python/modsupport.c
+++ b/Python/modsupport.c
@ -63,48 +63,84 @@ static PyObject *do_mkdict(const char**, va_list *, int, int, int);
 static PyObject *do_mkvalue(const char**, va_list *, int);


+static void
+do_ignore(const char **p_format, va_list *p_va, int endchar, int n, int flags)
+{
+    PyObject *v;
+    int i;
+    assert(PyErr_Occurred());
+    v = PyTuple_New(n);
+    for (i = 0; i < n; i++) {
+        PyObject *exception, *value, *tb, *w;
+
+        PyErr_Fetch(&exception, &value, &tb);
+        w = do_mkvalue(p_format, p_va, flags);
+        PyErr_Restore(exception, value, tb);
+        if (w != NULL) {
+            if (v != NULL) {
+                PyTuple_SET_ITEM(v, i, w);
+            }
+            else {
+                Py_DECREF(w);
+            }
+        }
+    }
+    Py_XDECREF(v);
+    if (**p_format != endchar) {
+        PyErr_SetString(PyExc_SystemError,
+                        "Unmatched paren in format");
+        return;
+    }
+    if (endchar)
+        ++*p_format;
+}
+
 static PyObject *
 do_mkdict(const char **p_format, va_list *p_va, int endchar, int n, int flags)
 {
    PyObject *d;
    int i;
-    int itemfailed = 0;
    if (n < 0)
        return NULL;
-    if ((d = PyDict_New()) == NULL)
+    if (n % 2) {
+        PyErr_SetString(PyExc_SystemError,
+                        "Bad dict format");
+        do_ignore(p_format, p_va, endchar, n, flags);
        return NULL;
+    }
    /* Note that we can't bail immediately on error as this will leak
       refcounts on any 'N' arguments. */
+    if ((d = PyDict_New()) == NULL) {
+        do_ignore(p_format, p_va, endchar, n, flags);
+        return NULL;
+    }
    for (i = 0; i < n; i+= 2) {
        PyObject *k, *v;
-        int err;
+
        k = do_mkvalue(p_format, p_va, flags);
        if (k == NULL) {
-            itemfailed = 1;
-            Py_INCREF(Py_None);
-            k = Py_None;
-        }
-        v = do_mkvalue(p_format, p_va, flags);
-        if (v == NULL) {
-            itemfailed = 1;
-            Py_INCREF(Py_None);
-            v = Py_None;
-        }
-        err = PyDict_SetItem(d, k, v);
-        Py_DECREF(k);
-        Py_DECREF(v);
-        if (err < 0 || itemfailed) {
+            do_ignore(p_format, p_va, endchar, n - i - 1, flags);
            Py_DECREF(d);
            return NULL;
        }
+        v = do_mkvalue(p_format, p_va, flags);
+        if (v == NULL || PyDict_SetItem(d, k, v) < 0) {
+            do_ignore(p_format, p_va, endchar, n - i - 2, flags);
+            Py_DECREF(k);
+            Py_XDECREF(v);
+            Py_DECREF(d);
+            return NULL;
+        }
+        Py_DECREF(k);
+        Py_DECREF(v);
    }
-    if (d != NULL && **p_format != endchar) {
+    if (**p_format != endchar) {
        Py_DECREF(d);
-        d = NULL;
        PyErr_SetString(PyExc_SystemError,
                        "Unmatched paren in format");
+        return NULL;
    }
-    else if (endchar)
+    if (endchar)
        ++*p_format;
    return d;
 }
@ -114,29 +150,24 @@ do_mklist(const char **p_format, va_list *p_va, int endchar, int n, int flags)
 {
    PyObject *v;
    int i;
-    int itemfailed = 0;
    if (n < 0)
        return NULL;
-    v = PyList_New(n);
-    if (v == NULL)
-        return NULL;
    /* Note that we can't bail immediately on error as this will leak
       refcounts on any 'N' arguments. */
+    v = PyList_New(n);
+    if (v == NULL) {
+        do_ignore(p_format, p_va, endchar, n, flags);
+        return NULL;
+    }
    for (i = 0; i < n; i++) {
        PyObject *w = do_mkvalue(p_format, p_va, flags);
        if (w == NULL) {
-            itemfailed = 1;
-            Py_INCREF(Py_None);
-            w = Py_None;
+            do_ignore(p_format, p_va, endchar, n - i - 1, flags);
+            Py_DECREF(v);
+            return NULL;
        }
        PyList_SET_ITEM(v, i, w);
    }
-
-    if (itemfailed) {
-        /* do_mkvalue() should have already set an error */
-        Py_DECREF(v);
-        return NULL;
-    }
    if (**p_format != endchar) {
        Py_DECREF(v);
        PyErr_SetString(PyExc_SystemError,
@ -153,37 +184,23 @@ do_mktuple(const char **p_format, va_list *p_va, int endchar, int n, int flags)
 {
    PyObject *v;
    int i;
-    int itemfailed = 0;
    if (n < 0)
        return NULL;
-    if ((v = PyTuple_New(n)) == NULL)
-        return NULL;
    /* Note that we can't bail immediately on error as this will leak
       refcounts on any 'N' arguments. */
+    if ((v = PyTuple_New(n)) == NULL) {
+        do_ignore(p_format, p_va, endchar, n, flags);
+        return NULL;
+    }
    for (i = 0; i < n; i++) {
-        PyObject *w;
-
-        if (itemfailed) {
-            PyObject *exception, *value, *tb;
-            PyErr_Fetch(&exception, &value, &tb);
-            w = do_mkvalue(p_format, p_va, flags);
-            PyErr_Restore(exception, value, tb);
-        }
-        else {
-            w = do_mkvalue(p_format, p_va, flags);
-        }
+        PyObject *w = do_mkvalue(p_format, p_va, flags);
        if (w == NULL) {
-            itemfailed = 1;
-            Py_INCREF(Py_None);
-            w = Py_None;
+            do_ignore(p_format, p_va, endchar, n - i - 1, flags);
+            Py_DECREF(v);
+            return NULL;
        }
        PyTuple_SET_ITEM(v, i, w);
    }
-    if (itemfailed) {
-        /* do_mkvalue() should have already set an error */
-        Py_DECREF(v);
-        return NULL;
-    }
    if (**p_format != endchar) {
        Py_DECREF(v);
        PyErr_SetString(PyExc_SystemError,