[Rpm-maint] [PATCH] handle errors when constructing lists in the Python bindings

David Malcolm dmalcolm at redhat.com
Fri Dec 16 03:24:19 UTC 2011


Various functions in the Python bindings construct lists of objects,
but assume that all calls succeed.

  python/header-py.c: hdrKeyList
  python/rpmps-py.c: rpmps_AsList
  python/rpmtd-py.c: rpmtd_AsPyobj
  python/spec-py.c: spec_get_sources
  python/spec-py.c: spec_get_packages

Each of these could segfault under low-memory conditions: if the PyList_New()
call fails, PyList_Append(NULL, item ) will segfault.
Similarly, although Py_List_Append(list, NULL) is safe, Py_DECREF(NULL) will
segfault.

Add error-handling for both cases to each function.

Caveat: compiles, but is not tested.

Found using an experimental static analysis tool I've been writing:
  http://gcc-python-plugin.readthedocs.org/en/latest/cpychecker.html
which is part of https://fedorahosted.org/gcc-python-plugin

A sample HTML report from the tool on one of these cases (specifically
hdrKeyList) can be seen here:
  http://fedorapeople.org/~dmalcolm/gcc-python-plugin/2011-12-15/header-py.c.hdrKeyList-refcount-errors.html
---
 python/header-py.c |   15 +++++++++++++--
 python/rpmps-py.c  |   16 ++++++++++++++--
 python/rpmtd-py.c  |    7 +++++++
 python/spec-py.c   |   27 ++++++++++++++++++++++++---
 4 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/python/header-py.c b/python/header-py.c
index cef457b..9e245aa 100644
--- a/python/header-py.c
+++ b/python/header-py.c
@@ -132,12 +132,23 @@ struct hdrObject_s {
 
 static PyObject * hdrKeyList(hdrObject * s)
 {
-    PyObject * keys = PyList_New(0);
-    HeaderIterator hi = headerInitIterator(s->h);
+    PyObject * keys;
+    HeaderIterator hi;
     rpmTagVal tag;
 
+    keys = PyList_New(0);
+    if (!keys) {
+        return NULL;
+    }
+
+    hi = headerInitIterator(s->h);
     while ((tag = headerNextTag(hi)) != RPMTAG_NOT_FOUND) {
 	PyObject *to = PyInt_FromLong(tag);
+        if (!to) {
+            headerFreeIterator(hi);
+            Py_DECREF(keys);
+            return NULL;
+        }
 	PyList_Append(keys, to);
 	Py_DECREF(to);
     }
diff --git a/python/rpmps-py.c b/python/rpmps-py.c
index 8b178fa..bdc899a 100644
--- a/python/rpmps-py.c
+++ b/python/rpmps-py.c
@@ -125,12 +125,24 @@ PyObject *rpmprob_Wrap(PyTypeObject *subtype, rpmProblem prob)
 
 PyObject *rpmps_AsList(rpmps ps)
 {
-    PyObject *problems = PyList_New(0);
-    rpmpsi psi = rpmpsInitIterator(ps);
+    PyObject *problems;
+    rpmpsi psi;
     rpmProblem prob;
 
+    problems = PyList_New(0);
+    if (!problems) {
+        return NULL;
+    }
+
+    psi = rpmpsInitIterator(ps);
+
     while ((prob = rpmpsiNext(psi))) {
         PyObject *pyprob = rpmprob_Wrap(&rpmProblem_Type, prob);
+        if (!pyprob) {
+            Py_DECREF(problems);
+            rpmpsFreeIterator(psi);
+            return NULL;
+        }
         PyList_Append(problems, pyprob);
         Py_DECREF(pyprob);
     }
diff --git a/python/rpmtd-py.c b/python/rpmtd-py.c
index 8232127..026f6c7 100644
--- a/python/rpmtd-py.c
+++ b/python/rpmtd-py.c
@@ -44,8 +44,15 @@ PyObject *rpmtd_AsPyobj(rpmtd td)
     
     if (array) {
 	res = PyList_New(0);
+        if (!res) {
+            return NULL;
+        }
 	while (rpmtdNext(td) >= 0) {
 	    PyObject *item = rpmtd_ItemAsPyobj(td, tclass);
+            if (!item) {
+                Py_DECREF(res);
+                return NULL;
+            }
 	    PyList_Append(res, item);
 	    Py_DECREF(item);
 	}
diff --git a/python/spec-py.c b/python/spec-py.c
index 49b9e1d..1850a50 100644
--- a/python/spec-py.c
+++ b/python/spec-py.c
@@ -151,15 +151,24 @@ static PyObject * spec_get_clean(specObject * s, void *closure)
 
 static PyObject * spec_get_sources(specObject *s, void *closure)
 {
-    PyObject *sourceList = PyList_New(0);
+    PyObject *sourceList;
     rpmSpecSrc source;
 
+    sourceList = PyList_New(0);
+    if (!sourceList) {
+        return NULL;
+    }
+
     rpmSpecSrcIter iter = rpmSpecSrcIterInit(s->spec);
     while ((source = rpmSpecSrcIterNext(iter)) != NULL) {
 	PyObject *srcUrl = Py_BuildValue("(sii)",
 				rpmSpecSrcFilename(source, 1),
 				rpmSpecSrcNum(source),
 				rpmSpecSrcFlags(source)); 
+        if (!srcUrl) {
+            Py_DECREF(sourceList);
+            return NULL;
+        }
 	PyList_Append(sourceList, srcUrl);
 	Py_DECREF(srcUrl);
     } 
@@ -172,11 +181,23 @@ static PyObject * spec_get_sources(specObject *s, void *closure)
 static PyObject * spec_get_packages(specObject *s, void *closure)
 {
     rpmSpecPkg pkg;
-    PyObject *pkgList = PyList_New(0);
-    rpmSpecPkgIter iter = rpmSpecPkgIterInit(s->spec);
+    PyObject *pkgList;
+    rpmSpecPkgIter iter;
+
+    pkgList = PyList_New(0);
+    if (!pkgList) {
+        return NULL;
+    }
+
+    iter = rpmSpecPkgIterInit(s->spec);
 
     while ((pkg = rpmSpecPkgIterNext(iter)) != NULL) {
 	PyObject *po = specPkg_Wrap(&specPkg_Type, pkg);
+        if (!po) {
+            rpmSpecPkgIterFree(iter);
+            Py_DECREF(pkgList);
+            return NULL;
+        }
 	PyList_Append(pkgList, po);
 	Py_DECREF(po);
     }
-- 
1.7.6.2




More information about the Rpm-maint mailing list