[Bug #1406906] [PATCH] python: fix segmentation fault when setting non UTF-8 strings
When constructing the returned objects, check the return value of Python APIs. A RuntimeError will be raised on failure pointing to the problematic entry and the field name. Signed-off-by: Matteo Cafasso <noxdafox@gmail.com> --- generator/python.ml | 143 +++++++++++++++++++++++++++++++++++----------------- 1 file changed, 97 insertions(+), 46 deletions(-) diff --git a/generator/python.ml b/generator/python.ml index 11dc48102..7d86131b1 100644 --- a/generator/python.ml +++ b/generator/python.ml @@ -152,12 +152,20 @@ and generate_python_structs () = pr "PyObject *\n"; pr "guestfs_int_py_put_%s_list (struct guestfs_%s_list *%ss)\n" typ typ typ; pr "{\n"; - pr " PyObject *list;\n"; + pr " PyObject *list, *element;\n"; pr " size_t i;\n"; pr "\n"; pr " list = PyList_New (%ss->len);\n" typ; - pr " for (i = 0; i < %ss->len; ++i)\n" typ; - pr " PyList_SetItem (list, i, guestfs_int_py_put_%s (&%ss->val[i]));\n" typ typ; + pr " if (list == NULL) {\n"; + pr " PyErr_SetString (PyExc_RuntimeError, \"PyList_New\");\n"; + pr " return NULL;\n"; + pr " }\n"; + pr " for (i = 0; i < %ss->len; ++i) {\n" typ; + pr " element = guestfs_int_py_put_%s (&%ss->val[i]);\n" typ typ; + pr " if (element == NULL)\n"; + pr " return NULL;\n"; + pr " PyList_SetItem (list, i, element);\n"; + pr " }\n"; pr " return list;\n"; pr "};\n"; pr "#endif\n"; @@ -171,73 +179,112 @@ and generate_python_structs () = pr "PyObject *\n"; pr "guestfs_int_py_put_%s (struct guestfs_%s *%s)\n" typ typ typ; pr "{\n"; - pr " PyObject *dict;\n"; + pr " PyObject *dict, *value;\n"; pr "\n"; pr " dict = PyDict_New ();\n"; + pr " if (dict == NULL) {\n"; + pr " PyErr_SetString (PyExc_RuntimeError, \"PyDict_New\");\n"; + pr " return NULL;\n"; + pr " }\n"; List.iter ( function | name, FString -> - pr " PyDict_SetItemString (dict, \"%s\",\n" name; pr "#ifdef HAVE_PYSTRING_ASSTRING\n"; - pr " PyString_FromString (%s->%s));\n" - typ name; + pr " value = PyString_FromString (%s->%s);\n" typ name; pr "#else\n"; - pr " PyUnicode_FromString (%s->%s));\n" - typ name; - pr "#endif\n" + pr " value = PyUnicode_FromString (%s->%s);\n" typ name; + pr "#endif\n"; + pr " if (value == NULL) {\n"; + pr " PyErr_SetString (PyExc_RuntimeError,\n"; + pr " \"Error setting %s.%s\");\n" typ name; + pr " return NULL;\n"; + pr " }\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; | name, FBuffer -> - pr " PyDict_SetItemString (dict, \"%s\",\n" name; pr "#ifdef HAVE_PYSTRING_ASSTRING\n"; - pr " PyString_FromStringAndSize (%s->%s, %s->%s_len));\n" + pr " value = PyString_FromStringAndSize (%s->%s, %s->%s_len);\n" typ name typ name; pr "#else\n"; - pr " PyBytes_FromStringAndSize (%s->%s, %s->%s_len));\n" + pr " value = PyBytes_FromStringAndSize (%s->%s, %s->%s_len);\n" typ name typ name; - pr "#endif\n" + pr "#endif\n"; + pr " if (value == NULL) {\n"; + pr " PyErr_SetString (PyExc_RuntimeError,\n"; + pr " \"Error setting %s.%s\");\n" typ name; + pr " return NULL;\n"; + pr " }\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; | name, FUUID -> - pr " PyDict_SetItemString (dict, \"%s\",\n" name; pr "#ifdef HAVE_PYSTRING_ASSTRING\n"; - pr " PyString_FromStringAndSize (%s->%s, 32));\n" - typ name; + pr " value = PyString_FromStringAndSize (%s->%s, 32);\n" typ name; pr "#else\n"; - pr " PyBytes_FromStringAndSize (%s->%s, 32));\n" - typ name; - pr "#endif\n" + pr " value = PyBytes_FromStringAndSize (%s->%s, 32);\n" typ name; + pr "#endif\n"; + pr " if (value == NULL) {\n"; + pr " PyErr_SetString (PyExc_RuntimeError,\n"; + pr " \"Error setting %s.%s\");\n" typ name; + pr " return NULL;\n"; + pr " }\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; | name, (FBytes|FUInt64) -> - pr " PyDict_SetItemString (dict, \"%s\",\n" name; - pr " PyLong_FromUnsignedLongLong (%s->%s));\n" - typ name + pr " value = PyLong_FromUnsignedLongLong (%s->%s);\n" typ name; + pr " if (value == NULL) {\n"; + pr " PyErr_SetString (PyExc_RuntimeError,\n"; + pr " \"Error setting %s.%s\");\n" typ name; + pr " return NULL;\n"; + pr " }\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; | name, FInt64 -> - pr " PyDict_SetItemString (dict, \"%s\",\n" name; - pr " PyLong_FromLongLong (%s->%s));\n" - typ name + pr " value = PyLong_FromLongLong (%s->%s);\n" typ name; + pr " if (value == NULL) {\n"; + pr " PyErr_SetString (PyExc_RuntimeError,\n"; + pr " \"Error setting %s.%s\");\n" typ name; + pr " return NULL;\n"; + pr " }\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; | name, FUInt32 -> - pr " PyDict_SetItemString (dict, \"%s\",\n" name; - pr " PyLong_FromUnsignedLong (%s->%s));\n" - typ name + pr " value = PyLong_FromUnsignedLong (%s->%s);\n" typ name; + pr " if (value == NULL) {\n"; + pr " PyErr_SetString (PyExc_RuntimeError,\n"; + pr " \"Error setting %s.%s\");\n" typ name; + pr " return NULL;\n"; + pr " }\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; | name, FInt32 -> - pr " PyDict_SetItemString (dict, \"%s\",\n" name; - pr " PyLong_FromLong (%s->%s));\n" - typ name + pr " value = PyLong_FromLong (%s->%s);\n" typ name; + pr " if (value == NULL) {\n"; + pr " PyErr_SetString (PyExc_RuntimeError,\n"; + pr " \"Error setting %s.%s\");\n" typ name; + pr " return NULL;\n"; + pr " }\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; | name, FOptPercent -> - pr " if (%s->%s >= 0)\n" typ name; - pr " PyDict_SetItemString (dict, \"%s\",\n" name; - pr " PyFloat_FromDouble ((double) %s->%s));\n" - typ name; + pr " if (%s->%s >= 0) {\n" typ name; + pr " value = PyFloat_FromDouble ((double) %s->%s);\n" typ name; + pr " if (value == NULL) {\n"; + pr " PyErr_SetString (PyExc_RuntimeError,\n"; + pr " \"Error setting %s.%s\");\n" typ name; + pr " return NULL;\n"; + pr " }\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; + pr " }\n"; pr " else {\n"; pr " Py_INCREF (Py_None);\n"; pr " PyDict_SetItemString (dict, \"%s\", Py_None);\n" name; pr " }\n" | name, FChar -> pr "#ifdef HAVE_PYSTRING_ASSTRING\n"; - pr " PyDict_SetItemString (dict, \"%s\",\n" name; - pr " PyString_FromStringAndSize (&%s->%s, 1));\n" - typ name; + pr " value = PyString_FromStringAndSize (&%s->%s, 1);\n" typ name; pr "#else\n"; - pr " PyDict_SetItemString (dict, \"%s\",\n" name; - pr " PyUnicode_FromStringAndSize (&%s->%s, 1));\n" + pr " value = PyUnicode_FromStringAndSize (&%s->%s, 1);\n" typ name; - pr "#endif\n" + pr "#endif\n"; + pr " if (value == NULL) {\n"; + pr " PyErr_SetString (PyExc_RuntimeError,\n"; + pr " \"Error setting %s.%s\");\n" typ name; + pr " return NULL;\n"; + pr " }\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; ) cols; pr " return dict;\n"; pr "};\n"; @@ -517,16 +564,20 @@ and generate_python_actions actions () = pr " if (py_r == NULL) goto out;\n"; | RStringList _ -> pr " py_r = guestfs_int_py_put_string_list (r);\n"; - pr " guestfs_int_free_string_list (r);\n" + pr " guestfs_int_free_string_list (r);\n"; + pr " if (py_r == NULL) goto out;\n"; | RStruct (_, typ) -> pr " py_r = guestfs_int_py_put_%s (r);\n" typ; - pr " guestfs_free_%s (r);\n" typ + pr " guestfs_free_%s (r);\n" typ; + pr " if (py_r == NULL) goto out;\n"; | RStructList (_, typ) -> pr " py_r = guestfs_int_py_put_%s_list (r);\n" typ; - pr " guestfs_free_%s_list (r);\n" typ + pr " guestfs_free_%s_list (r);\n" typ; + pr " if (py_r == NULL) goto out;\n"; | RHashtable n -> pr " py_r = guestfs_int_py_put_table (r);\n"; - pr " guestfs_int_free_string_list (r);\n" + pr " guestfs_int_free_string_list (r);\n"; + pr " if (py_r == NULL) goto out;\n"; | RBufferOut _ -> pr "#ifdef HAVE_PYSTRING_ASSTRING\n"; pr " py_r = PyString_FromStringAndSize (r, size);\n"; -- 2.11.0
On Tuesday, 25 April 2017 22:25:20 CEST Matteo Cafasso wrote:
When constructing the returned objects, check the return value of Python APIs.
A RuntimeError will be raised on failure pointing to the problematic entry and the field name.
Signed-off-by: Matteo Cafasso <noxdafox@gmail.com> --- generator/python.ml | 143 +++++++++++++++++++++++++++++++++++----------------- 1 file changed, 97 insertions(+), 46 deletions(-)
diff --git a/generator/python.ml b/generator/python.ml index 11dc48102..7d86131b1 100644 --- a/generator/python.ml +++ b/generator/python.ml @@ -152,12 +152,20 @@ and generate_python_structs () = pr "PyObject *\n"; pr "guestfs_int_py_put_%s_list (struct guestfs_%s_list *%ss)\n" typ typ typ; pr "{\n"; - pr " PyObject *list;\n"; + pr " PyObject *list, *element;\n"; pr " size_t i;\n"; pr "\n"; pr " list = PyList_New (%ss->len);\n" typ; - pr " for (i = 0; i < %ss->len; ++i)\n" typ; - pr " PyList_SetItem (list, i, guestfs_int_py_put_%s (&%ss->val[i]));\n" typ typ; + pr " if (list == NULL) {\n"; + pr " PyErr_SetString (PyExc_RuntimeError, \"PyList_New\");\n";
This should most probably: return PyErr_NoMemory(); as it sets the right exception for "no memory" situations. Also all the other NULL checks in this patch for PySomething*() APIs should do the same.
+ pr " return NULL;\n"; + pr " }\n"; + pr " for (i = 0; i < %ss->len; ++i) {\n" typ; + pr " element = guestfs_int_py_put_%s (&%ss->val[i]);\n" typ typ; + pr " if (element == NULL)\n"; + pr " return NULL;\n";
This leaks 'list'. Similar situation also in other parts of this patch. Also, I guess also the other guestfs_int_* functions in python/handle.c need similar checks/changes? Moreover, can you please add a regression test for this? I.e. a new python/t/test8xxSomething.py, formatted as PEP-8. See the section "ADDING A NEW LANGUAGE BINDING" in the docs/guestfs-hacking documentation for the numbering of tests. Thanks, -- Pino Toscano
participants (2)
-
Matteo Cafasso -
Pino Toscano