pcp
[Top] [All Lists]

[PATCH] Fix memory corruption in python support

To: pcp <pcp@xxxxxxxxxxx>
Subject: [PATCH] Fix memory corruption in python support
From: David Smith <dsmith@xxxxxxxxxx>
Date: Thu, 02 Oct 2014 09:50:14 -0500
Delivered-to: pcp@xxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.1
Here's a patch that fixes some memory corruption I've seen in pcp's
python support. Here's a note from the python documentation
(<https://docs.python.org/2/c-api/arg.html>):

====
Note that any Python object references which are provided to the caller
[of PyArg_ParseTuple(), PyArg_ParseTupleAndKeywords(), or PyArg_Parse()]
are borrowed references; do not decrement their reference count!
====

Unfortunately, pcp's python support was decrementing reference counts in
some cases. I went through and audited the calls to PyArg_ParseTuple(),
PyArg_ParseTupleAndKeywords(), and PyArg_Parse() and believe I've fixed
the reference count issues.

This patch fixed the memory corruption I was seeing.

---
diff --git a/src/python/pmapi.c b/src/python/pmapi.c
index 6886214..13a6dcd 100644
--- a/src/python/pmapi.c
+++ b/src/python/pmapi.c
@@ -682,6 +682,9 @@ getOptionsFromList(PyObject *self, PyObject *args,
PyObject *keywords)
     PyObject *pyargv = NULL;
     char *keyword_list[] = {"argv", NULL};

+    // Note that PyArg_ParseTupleAndKeywords() returns a "borrowed"
+    // reference, so there is no need to decrement a reference to
+    // pyargv.
     if (!PyArg_ParseTupleAndKeywords(args, keywords,
                        "O:pmGetOptionsFromList", keyword_list, &pyargv))
        return NULL;
@@ -691,17 +694,14 @@ getOptionsFromList(PyObject *self, PyObject *args,
PyObject *keywords)

     if (!PyList_Check(pyargv)) {
        PyErr_SetString(PyExc_TypeError, "pmGetOptionsFromList uses a list");
-       Py_DECREF(pyargv);
        return NULL;
     }

     if ((argc = PyList_GET_SIZE(pyargv)) <= 0) {
-       Py_DECREF(pyargv);
        return Py_BuildValue("i", 0);
     }

     if ((argv = malloc(argc * sizeof(char *))) == NULL) {
-       Py_DECREF(pyargv);
        return PyErr_NoMemory();
     }

@@ -718,7 +718,6 @@ getOptionsFromList(PyObject *self, PyObject *args,
PyObject *keywords)
          */
        if (i == 0 && (string = strdup(string)) == NULL) {
            free(argv);
-           Py_DECREF(pyargv);
            return PyErr_NoMemory();
        }
        argv[i] = string;
diff --git a/src/python/pmda.c b/src/python/pmda.c
index 73e3d92..f0c4845 100644
--- a/src/python/pmda.c
+++ b/src/python/pmda.c
@@ -124,6 +124,11 @@ namespace_refresh(PyObject *self, PyObject *args,
PyObject *keywords)
                         "O:namespace_refresh", keyword_list, &pmns_dict))
         return NULL;
     if (pmns_dict) {
+       // PyArg_ParseTupleAndKeywords() returns a "borrowed"
+       // reference. Since we're going to keep this object around for
+       // use later, increase its reference count.
+       Py_INCREF(pmns_dict);
+
         if (!PyDict_Check(pmns_dict)) {
             __pmNotifyErr(LOG_ERR,
                 "attempted to refresh namespace with non-dict type");
@@ -153,6 +158,11 @@ pmid_oneline_refresh(PyObject *self, PyObject
*args, PyObject *keywords)
         return NULL;

     if (pmid_oneline_dict) {
+       // PyArg_ParseTupleAndKeywords() returns a "borrowed"
+       // reference. Since we're going to keep this object around for
+       // use later, increase its reference count.
+       Py_INCREF(pmid_oneline_dict);
+
         if (!PyDict_Check(pmid_oneline_dict)) {
             __pmNotifyErr(LOG_ERR,
                 "attempted to refresh pmid oneline help with non-dict
type");
@@ -180,6 +190,11 @@ pmid_longtext_refresh(PyObject *self, PyObject
*args, PyObject *keywords)
         return NULL;

     if (pmid_longtext_dict) {
+       // PyArg_ParseTupleAndKeywords() returns a "borrowed"
+       // reference. Since we're going to keep this object around for
+       // use later, increase its reference count.
+       Py_INCREF(pmid_longtext_dict);
+
         if (!PyDict_Check(pmid_longtext_dict)) {
             __pmNotifyErr(LOG_ERR,
                 "attempted to refresh pmid long help with non-dict type");
@@ -207,6 +222,11 @@ indom_oneline_refresh(PyObject *self, PyObject
*args, PyObject *keywords)
         return NULL;

     if (indom_oneline_dict) {
+       // PyArg_ParseTupleAndKeywords() returns a "borrowed"
+       // reference. Since we're going to keep this object around for
+       // use later, increase its reference count.
+       Py_INCREF(indom_oneline_dict);
+
         if (!PyDict_Check(indom_oneline_dict)) {
             __pmNotifyErr(LOG_ERR,
                 "attempted to refresh indom oneline help with non-dict
type");
@@ -234,6 +254,11 @@ indom_longtext_refresh(PyObject *self, PyObject
*args, PyObject *keywords)
         return NULL;

     if (indom_longtext_dict) {
+       // PyArg_ParseTupleAndKeywords() returns a "borrowed"
+       // reference. Since we're going to keep this object around for
+       // use later, increase its reference count.
+       Py_INCREF(indom_longtext_dict);
+
         if (!PyDict_Check(indom_longtext_dict)) {
             __pmNotifyErr(LOG_ERR,
                 "attempted to refresh indom long help with non-dict type");
---


-- 
David Smith
dsmith@xxxxxxxxxx
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)

<Prev in Thread] Current Thread [Next in Thread>