Skip to content

Commit 12cf60c

Browse files
serhiy-storchakaserhiy-storchaka
committed
Issue python#26168: Fixed possible refleaks in failing Py_BuildValue() with the "N"
format unit.
1 parent 6546d7c commit 12cf60c

4 files changed

Lines changed: 172 additions & 43 deletions

File tree

‎Lib/test/test_capi.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@
1616
# Skip this test if the _testcapi module isn't available.
1717
_testcapi = support.import_module('_testcapi')
1818

19+
class CAPITest(unittest.TestCase):
20+
21+
def test_buildvalue_N(self):
22+
_testcapi.test_buildvalue_N()
23+
1924

2025
@unittest.skipUnless(threading, 'Threading required for this test.')
2126
class TestPendingCalls(unittest.TestCase):
@@ -132,7 +137,7 @@ def test_main():
132137
except _testcapi.error:
133138
raise support.TestFailed, sys.exc_info()[1]
134139

135-
support.run_unittest(TestPendingCalls, TestThreadState)
140+
support.run_unittest(CAPITest, TestPendingCalls, TestThreadState)
136141

137142
if __name__ == "__main__":
138143
test_main()

‎Misc/NEWS‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ What's New in Python 2.7.12?
1010
Core and Builtins
1111
-----------------
1212

13+
- Issue #26168: Fixed possible refleaks in failing Py_BuildValue() with the "N"
14+
format unit.
15+
1316
- Issue #27039: Fixed bytearray.remove() for values greater than 127. Patch by
1417
Joe Jevnik.
1518

‎Modules/_testcapimodule.c‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -930,6 +930,100 @@ test_L_code(PyObject *self)
930930

931931
#endif /* ifdef HAVE_LONG_LONG */
932932

933+
static PyObject *
934+
return_none(void *unused)
935+
{
936+
Py_RETURN_NONE;
937+
}
938+
939+
static PyObject *
940+
raise_error(void *unused)
941+
{
942+
PyErr_SetNone(PyExc_ValueError);
943+
return NULL;
944+
}
945+
946+
static int
947+
test_buildvalue_N_error(const char *fmt)
948+
{
949+
PyObject *arg, *res;
950+
951+
arg = PyList_New(0);
952+
if (arg == NULL) {
953+
return -1;
954+
}
955+
956+
Py_INCREF(arg);
957+
res = Py_BuildValue(fmt, return_none, NULL, arg);
958+
if (res == NULL) {
959+
return -1;
960+
}
961+
Py_DECREF(res);
962+
if (Py_REFCNT(arg) != 1) {
963+
PyErr_Format(TestError, "test_buildvalue_N: "
964+
"arg was not decrefed in successful "
965+
"Py_BuildValue(\"%s\")", fmt);
966+
return -1;
967+
}
968+
969+
Py_INCREF(arg);
970+
res = Py_BuildValue(fmt, raise_error, NULL, arg);
971+
if (res != NULL || !PyErr_Occurred()) {
972+
PyErr_Format(TestError, "test_buildvalue_N: "
973+
"Py_BuildValue(\"%s\") didn't complain", fmt);
974+
return -1;
975+
}
976+
PyErr_Clear();
977+
if (Py_REFCNT(arg) != 1) {
978+
PyErr_Format(TestError, "test_buildvalue_N: "
979+
"arg was not decrefed in failed "
980+
"Py_BuildValue(\"%s\")", fmt);
981+
return -1;
982+
}
983+
Py_DECREF(arg);
984+
return 0;
985+
}
986+
987+
static PyObject *
988+
test_buildvalue_N(PyObject *self, PyObject *noargs)
989+
{
990+
PyObject *arg, *res;
991+
992+
arg = PyList_New(0);
993+
if (arg == NULL) {
994+
return NULL;
995+
}
996+
Py_INCREF(arg);
997+
res = Py_BuildValue("N", arg);
998+
if (res == NULL) {
999+
return NULL;
1000+
}
1001+
if (res != arg) {
1002+
return raiseTestError("test_buildvalue_N",
1003+
"Py_BuildValue(\"N\") returned wrong result");
1004+
}
1005+
if (Py_REFCNT(arg) != 2) {
1006+
return raiseTestError("test_buildvalue_N",
1007+
"arg was not decrefed in Py_BuildValue(\"N\")");
1008+
}
1009+
Py_DECREF(res);
1010+
Py_DECREF(arg);
1011+
1012+
if (test_buildvalue_N_error("O&N") < 0)
1013+
return NULL;
1014+
if (test_buildvalue_N_error("(O&N)") < 0)
1015+
return NULL;
1016+
if (test_buildvalue_N_error("[O&N]") < 0)
1017+
return NULL;
1018+
if (test_buildvalue_N_error("{O&N}") < 0)
1019+
return NULL;
1020+
if (test_buildvalue_N_error("{()O&(())N}") < 0)
1021+
return NULL;
1022+
1023+
Py_RETURN_NONE;
1024+
}
1025+
1026+
9331027
static PyObject *
9341028
get_args(PyObject *self, PyObject *args)
9351029
{
@@ -2414,6 +2508,7 @@ static PyMethodDef TestMethods[] = {
24142508
{"test_with_docstring", (PyCFunction)test_with_docstring, METH_NOARGS,
24152509
PyDoc_STR("This is a pretty normal docstring.")},
24162510

2511+
{"test_buildvalue_N", test_buildvalue_N, METH_NOARGS},
24172512
{"get_args", get_args, METH_VARARGS},
24182513
{"get_kwargs", (PyCFunction)get_kwargs, METH_VARARGS|METH_KEYWORDS},
24192514
{"getargs_tuple", getargs_tuple, METH_VARARGS},

‎Python/modsupport.c‎

Lines changed: 68 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -156,48 +156,83 @@ static PyObject *do_mkdict(const char**, va_list *, int, int, int);
156156
static PyObject *do_mkvalue(const char**, va_list *, int);
157157

158158

159+
static void
160+
do_ignore(const char **p_format, va_list *p_va, int endchar, int n, int flags)
161+
{
162+
PyObject *v;
163+
int i;
164+
assert(PyErr_Occurred());
165+
v = PyTuple_New(n);
166+
for (i = 0; i < n; i++) {
167+
PyObject *exception, *value, *tb, *w;
168+
PyErr_Fetch(&exception, &value, &tb);
169+
w = do_mkvalue(p_format, p_va, flags);
170+
PyErr_Restore(exception, value, tb);
171+
if (w != NULL) {
172+
if (v != NULL) {
173+
PyTuple_SET_ITEM(v, i, w);
174+
}
175+
else {
176+
Py_DECREF(w);
177+
}
178+
}
179+
}
180+
Py_XDECREF(v);
181+
if (**p_format != endchar) {
182+
PyErr_SetString(PyExc_SystemError,
183+
"Unmatched paren in format");
184+
return;
185+
}
186+
if (endchar)
187+
++*p_format;
188+
}
189+
159190
static PyObject *
160191
do_mkdict(const char **p_format, va_list *p_va, int endchar, int n, int flags)
161192
{
162193
PyObject *d;
163194
int i;
164-
int itemfailed = 0;
165195
if (n < 0)
166196
return NULL;
167-
if ((d = PyDict_New()) == NULL)
197+
if (n % 2) {
198+
PyErr_SetString(PyExc_SystemError,
199+
"Bad dict format");
200+
do_ignore(p_format, p_va, endchar, n, flags);
168201
return NULL;
202+
}
169203
/* Note that we can't bail immediately on error as this will leak
170204
refcounts on any 'N' arguments. */
205+
if ((d = PyDict_New()) == NULL) {
206+
do_ignore(p_format, p_va, endchar, n, flags);
207+
return NULL;
208+
}
171209
for (i = 0; i < n; i+= 2) {
172210
PyObject *k, *v;
173-
int err;
211+
174212
k = do_mkvalue(p_format, p_va, flags);
175213
if (k == NULL) {
176-
itemfailed = 1;
177-
Py_INCREF(Py_None);
178-
k = Py_None;
214+
do_ignore(p_format, p_va, endchar, n - i - 1, flags);
215+
Py_DECREF(d);
216+
return NULL;
179217
}
180218
v = do_mkvalue(p_format, p_va, flags);
181-
if (v == NULL) {
182-
itemfailed = 1;
183-
Py_INCREF(Py_None);
184-
v = Py_None;
185-
}
186-
err = PyDict_SetItem(d, k, v);
187-
Py_DECREF(k);
188-
Py_DECREF(v);
189-
if (err < 0 || itemfailed) {
219+
if (v == NULL || PyDict_SetItem(d, k, v) < 0) {
220+
do_ignore(p_format, p_va, endchar, n - i - 2, flags);
221+
Py_DECREF(k);
222+
Py_XDECREF(v);
190223
Py_DECREF(d);
191224
return NULL;
192225
}
226+
Py_DECREF(k);
227+
Py_DECREF(v);
193228
}
194-
if (d != NULL && **p_format != endchar) {
229+
if (**p_format != endchar) {
195230
Py_DECREF(d);
196-
d = NULL;
197231
PyErr_SetString(PyExc_SystemError,
198232
"Unmatched paren in format");
233+
return NULL;
199234
}
200-
else if (endchar)
235+
if (endchar)
201236
++*p_format;
202237
return d;
203238
}
@@ -207,29 +242,24 @@ do_mklist(const char **p_format, va_list *p_va, int endchar, int n, int flags)
207242
{
208243
PyObject *v;
209244
int i;
210-
int itemfailed = 0;
211245
if (n < 0)
212246
return NULL;
213-
v = PyList_New(n);
214-
if (v == NULL)
215-
return NULL;
216247
/* Note that we can't bail immediately on error as this will leak
217248
refcounts on any 'N' arguments. */
249+
v = PyList_New(n);
250+
if (v == NULL) {
251+
do_ignore(p_format, p_va, endchar, n, flags);
252+
return NULL;
253+
}
218254
for (i = 0; i < n; i++) {
219255
PyObject *w = do_mkvalue(p_format, p_va, flags);
220256
if (w == NULL) {
221-
itemfailed = 1;
222-
Py_INCREF(Py_None);
223-
w = Py_None;
257+
do_ignore(p_format, p_va, endchar, n - i - 1, flags);
258+
Py_DECREF(v);
259+
return NULL;
224260
}
225261
PyList_SET_ITEM(v, i, w);
226262
}
227-
228-
if (itemfailed) {
229-
/* do_mkvalue() should have already set an error */
230-
Py_DECREF(v);
231-
return NULL;
232-
}
233263
if (**p_format != endchar) {
234264
Py_DECREF(v);
235265
PyErr_SetString(PyExc_SystemError,
@@ -257,27 +287,23 @@ do_mktuple(const char **p_format, va_list *p_va, int endchar, int n, int flags)
257287
{
258288
PyObject *v;
259289
int i;
260-
int itemfailed = 0;
261290
if (n < 0)
262291
return NULL;
263-
if ((v = PyTuple_New(n)) == NULL)
264-
return NULL;
265292
/* Note that we can't bail immediately on error as this will leak
266293
refcounts on any 'N' arguments. */
294+
if ((v = PyTuple_New(n)) == NULL) {
295+
do_ignore(p_format, p_va, endchar, n, flags);
296+
return NULL;
297+
}
267298
for (i = 0; i < n; i++) {
268299
PyObject *w = do_mkvalue(p_format, p_va, flags);
269300
if (w == NULL) {
270-
itemfailed = 1;
271-
Py_INCREF(Py_None);
272-
w = Py_None;
301+
do_ignore(p_format, p_va, endchar, n - i - 1, flags);
302+
Py_DECREF(v);
303+
return NULL;
273304
}
274305
PyTuple_SET_ITEM(v, i, w);
275306
}
276-
if (itemfailed) {
277-
/* do_mkvalue() should have already set an error */
278-
Py_DECREF(v);
279-
return NULL;
280-
}
281307
if (**p_format != endchar) {
282308
Py_DECREF(v);
283309
PyErr_SetString(PyExc_SystemError,

0 commit comments

Comments
 (0)