From eb2b6cd45022ac6779079cbc04f7715d0d22cfe4 Mon Sep 17 00:00:00 2001 From: Zackery Spytz Date: Sat, 8 Dec 2018 03:06:59 -0700 Subject: [PATCH 1/3] bpo-35441: Remove dead and buggy code related to PyList_SetItem() In _localemodule.c and selectmodule.c, remove dead code that would cause double decrefs if run. In addition, replace PyList_SetItem() with PyList_SET_ITEM() in cases where a new list is populated and there is no possibility of an error. --- Modules/_localemodule.c | 6 +----- Modules/arraymodule.c | 5 ++--- Modules/readline.c | 3 +-- Modules/selectmodule.c | 10 ++-------- PC/winreg.c | 10 +++++++--- Python/ceval.c | 4 ++-- Python/sysmodule.c | 2 +- 7 files changed, 16 insertions(+), 24 deletions(-) diff --git a/Modules/_localemodule.c b/Modules/_localemodule.c index 4202cc40141424..af1bc6e115605c 100644 --- a/Modules/_localemodule.c +++ b/Modules/_localemodule.c @@ -73,11 +73,7 @@ copy_grouping(const char* s) val = PyLong_FromLong(s[i]); if (!val) break; - if (PyList_SetItem(result, i, val)) { - Py_DECREF(val); - val = NULL; - break; - } + PyList_SET_ITEM(result, i, val); } while (s[i] != '\0' && s[i] != CHAR_MAX); if (!val) { diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 6a9ff3ec6252d9..9fc2d5bac3ddd1 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -1544,7 +1544,7 @@ array_array_fromlist(arrayobject *self, PyObject *list) if (array_resize(self, old_size + n) == -1) return NULL; for (i = 0; i < n; i++) { - PyObject *v = PyList_GetItem(list, i); + PyObject *v = PyList_GET_ITEM(list, i); if ((*self->ob_descr->setitem)(self, Py_SIZE(self) - n + i, v) != 0) { array_resize(self, old_size); @@ -1574,8 +1574,7 @@ array_array_tolist_impl(arrayobject *self) PyObject *v = getarrayitem((PyObject *)self, i); if (v == NULL) goto error; - if (PyList_SetItem(list, i, v) < 0) - goto error; + PyList_SET_ITEM(list, i, v); } return list; diff --git a/Modules/readline.c b/Modules/readline.c index 7f366aabfb4f43..57335fe911bff2 100644 --- a/Modules/readline.c +++ b/Modules/readline.c @@ -936,8 +936,7 @@ on_completion_display_matches_hook(char **matches, s = decode(matches[i+1]); if (s == NULL) goto error; - if (PyList_SetItem(m, i, s) == -1) - goto error; + PyList_SET_ITEM(m, i, s); } sub = decode(matches[0]); r = PyObject_CallFunction(readlinestate_global->completion_display_matches_hook, diff --git a/Modules/selectmodule.c b/Modules/selectmodule.c index fe69cd58dc6a28..7f62ab111d1f3e 100644 --- a/Modules/selectmodule.c +++ b/Modules/selectmodule.c @@ -693,10 +693,7 @@ select_poll_poll_impl(pollObject *self, PyObject *timeout_obj) goto error; } PyTuple_SET_ITEM(value, 1, num); - if ((PyList_SetItem(result_list, j, value)) == -1) { - Py_DECREF(value); - goto error; - } + PyList_SET_ITEM(result_list, j, value); i++; } return result_list; @@ -986,10 +983,7 @@ select_devpoll_poll_impl(devpollObject *self, PyObject *timeout_obj) Py_DECREF(num2); if (value == NULL) goto error; - if ((PyList_SetItem(result_list, i, value)) == -1) { - Py_DECREF(value); - goto error; - } + PyList_SET_ITEM(result_list, i, value); } return result_list; diff --git a/PC/winreg.c b/PC/winreg.c index 0198097a1c929f..4505c314d7b473 100644 --- a/PC/winreg.c +++ b/PC/winreg.c @@ -756,9 +756,13 @@ Reg2Py(BYTE *retDataBuf, DWORD retDataSize, DWORD typ) PyMem_Free(str); return NULL; } - PyList_SetItem(obData, - index, - PyUnicode_FromWideChar(str[index], len)); + PyObject *uni = PyUnicode_FromWideChar(str[index], len); + if (uni == NULL) { + Py_DECREF(obData); + PyMem_Free(str); + return NULL; + } + PyList_SET_ITEM(obData, index, uni); } PyMem_Free(str); diff --git a/Python/ceval.c b/Python/ceval.c index a4273adee48d81..7ffc68a1e1fc5c 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -5124,7 +5124,7 @@ getarray(long a[256]) Py_DECREF(l); return NULL; } - PyList_SetItem(l, i, x); + PyList_SET_ITEM(l, i, x); } for (i = 0; i < 256; i++) a[i] = 0; @@ -5146,7 +5146,7 @@ _Py_GetDXProfile(PyObject *self, PyObject *args) Py_DECREF(l); return NULL; } - PyList_SetItem(l, i, x); + PyList_SET_ITEM(l, i, x); } return l; #endif diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 0ca3de39f8bdc9..49fa3842b583b8 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2594,7 +2594,7 @@ makepathobject(const wchar_t *path, wchar_t delim) Py_DECREF(v); return NULL; } - PyList_SetItem(v, i, w); + PyList_SET_ITEM(v, i, w); if (*p == '\0') break; path = p+1; From 2a9bb21b5f4631a35133e963934043b9cf4354ae Mon Sep 17 00:00:00 2001 From: Zackery Spytz Date: Sat, 8 Dec 2018 03:49:11 -0700 Subject: [PATCH 2/3] Address the review comment. --- Modules/_localemodule.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/Modules/_localemodule.c b/Modules/_localemodule.c index af1bc6e115605c..96ca68af178c57 100644 --- a/Modules/_localemodule.c +++ b/Modules/_localemodule.c @@ -71,16 +71,13 @@ copy_grouping(const char* s) do { i++; val = PyLong_FromLong(s[i]); - if (!val) - break; + if (val == NULL) { + Py_DECREF(result); + return NULL; + } PyList_SET_ITEM(result, i, val); } while (s[i] != '\0' && s[i] != CHAR_MAX); - if (!val) { - Py_DECREF(result); - return NULL; - } - return result; } From 6268718b3a8da73b2fe721792b786d367e12ed43 Mon Sep 17 00:00:00 2001 From: Zackery Spytz Date: Sat, 8 Dec 2018 05:51:03 -0700 Subject: [PATCH 3/3] Check if the list changed size in the loop in array_array_fromlist() --- Modules/arraymodule.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 9fc2d5bac3ddd1..aa7a4fb23c688b 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -1550,6 +1550,12 @@ array_array_fromlist(arrayobject *self, PyObject *list) array_resize(self, old_size); return NULL; } + if (n != PyList_GET_SIZE(list)) { + PyErr_SetString(PyExc_RuntimeError, + "list changed size during iteration"); + array_resize(self, old_size); + return NULL; + } } } Py_RETURN_NONE;