[SSSD] [PATCH] pyhbac,pysss: fix reference leaks
Jakub Hrozek
jhrozek at redhat.com
Wed Oct 22 13:40:25 UTC 2014
On Tue, Oct 07, 2014 at 01:32:05PM +0200, Pavel Reichl wrote:
> Hello,
>
> please see attached patch.
>
> I haven't run the tool which found these leaks as setting it is not IMO
> quite so easy, but I have contacted the reporter and asked him if he is
> willing to participate at testing.
>
> Thanks!
> From 5b38c8e9e6b48cf86a6ed7d58b572710ca5c867a Mon Sep 17 00:00:00 2001
> From: Pavel Reichl <preichl at redhat.com>
> Date: Mon, 29 Sep 2014 16:40:53 +0100
> Subject: [PATCH] pyhbac,pysss: fix reference leaks
>
> Resolves:
> https://fedorahosted.org/sssd/ticket/1195
> ---
> src/python/pyhbac.c | 22 +++++++++++++---------
> src/python/pysss.c | 10 +++++++---
> 2 files changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/src/python/pyhbac.c b/src/python/pyhbac.c
> index e9dce9b01b78401516b5c21141be8007b4a94aec..dd345a6eb4db8ac6104251d5b9c8f11a160e280d 100644
> --- a/src/python/pyhbac.c
> +++ b/src/python/pyhbac.c
> @@ -139,14 +139,17 @@ sequence_as_string_list(PyObject *seq, const char *paramname)
>
> utf_item = get_utf8_string(item, p);
> if (utf_item == NULL) {
> + Py_DECREF(item);
> return NULL;
> }
>
> ret[i] = py_strdup(PyString_AsString(utf_item));
> Py_DECREF(utf_item);
> if (!ret[i]) {
> + Py_DECREF(item);
> return NULL;
> }
> + Py_DECREF(item);
item is returned from PySequence_GetItem(), the official documentation
says:
PyObject* PySequence_GetItem(PyObject *o, Py_ssize_t i)
Return value: New reference.
So this change is correct.
> }
>
> ret[i] = NULL;
> @@ -242,10 +245,7 @@ str_concat_sequence(PyObject *seq, const char *delim)
> if (item == NULL) goto fail;
>
> part = PyString_AsString(item);
> - if (part == NULL) {
> - Py_DECREF(item);
> - goto fail;
> - }
> + if (part == NULL) goto fail;
>
> if (s) {
> s = py_strcat_realloc(s, delim);
> @@ -260,7 +260,9 @@ str_concat_sequence(PyObject *seq, const char *delim)
> }
>
> return s;
> +
> fail:
> + Py_XDECREF(item);
Same here.
> PyMem_Free(s);
> return NULL;
> }
> @@ -269,11 +271,13 @@ fail:
> static void
> set_hbac_exception(PyObject *exc, struct hbac_info *error)
> {
> - PyErr_SetObject(exc,
> - Py_BuildValue(sss_py_const_p(char, "(i,s)"),
> - error->code,
> - error->rule_name ? \
> - error->rule_name : "no rule"));
> + PyObject *obj;
> +
> + obj = Py_BuildValue(sss_py_const_p(char, "(i,s)"), error->code,
> + error->rule_name ? error->rule_name : "no rule");
> +
> + PyErr_SetObject(exc, obj);
> + Py_XDECREF(obj);
This change would be nice to have unit tested, but I didn't see an
obvious way to do so..so I just did a poor man's test:
diff --git a/src/python/pyhbac.c b/src/python/pyhbac.c
index dd345a6..1d3308e 100644
--- a/src/python/pyhbac.c
+++ b/src/python/pyhbac.c
@@ -1604,6 +1604,7 @@ py_hbac_evaluate(HbacRequest *self, PyObject *args)
self->rule_name = NULL;
eres = hbac_evaluate(rules, hbac_req, &info);
+ eres = HBAC_EVAL_ERROR;
switch (eres) {
case HBAC_EVAL_ALLOW:
self->rule_name = sss_python_unicode_from_string(info->rule_name);
This raised an HbacError exception as I expected.
Valgrind didn't show any memory leaks /in the patched code/ before or
after the patch.
All patched code is tested and the changes look good to me.
ACK.
More information about the sssd-devel
mailing list