Matteo Cafasso
2017-Apr-25 20:25 UTC
[Libguestfs] [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
Pino Toscano
2017-May-03 16:02 UTC
Re: [Libguestfs] [Bug #1406906] [PATCH] python: fix segmentation fault when setting non UTF-8 strings
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
Apparently Analagous Threads
- [PATCH] python: check return value of Python APIs
- [PATCH] python: add simple wrappers for PyObject<->string functions
- [PATCH v2] python: add simple wrappers for PyObject<->string functions
- [PATCH v2] RHBZ#1406906: check return value of Python object functions
- [PATCH] RHBZ#1406906: check return value of Python object functions