On Fri, 2011-12-02 at 16:04 +0200, Alexander Bokovoy wrote:
> On Tue, 29 Nov 2011, Simo Sorce wrote:
> > Found a couple of memleaks while reviewing code.
> >
> > Attached.
> >
> > Simo.
> >
> > --
> > Simo Sorce * Red Hat, Inc * New York
>
> > >From 70840691e48e1ac89002499c08a9dd4fdcae7c50 Mon Sep 17 00:00:00 2001
> > From: Simo Sorce <sso...@redhat.com>
> > Date: Sun, 20 Nov 2011 20:50:11 -0500
> > Subject: [PATCH] ipa-kdb: fix memleaks in ipa_kdb_mspac.c
> >
> > ---
> > daemons/ipa-kdb/ipa_kdb_mspac.c | 13 +++++++------
> > 1 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c
> > b/daemons/ipa-kdb/ipa_kdb_mspac.c
> > index
> > cce1ca9060f0e03d525bb87d843bdd5811e9d20b..0c0da75ca8dbedf12e39e8ec5d87bfd4cd485d4a
> > 100644
> > --- a/daemons/ipa-kdb/ipa_kdb_mspac.c
> > +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
> > @@ -466,7 +466,7 @@ static krb5_error_code ipadb_get_pac(krb5_context
> > kcontext,
> > TALLOC_CTX *tmpctx;
> > struct ipadb_e_data *ied;
> > struct ipadb_context *ipactx;
> > - LDAPMessage *results;
> > + LDAPMessage *results = NULL;
> > LDAPMessage *lentry;
> > DATA_BLOB pac_data;
> > krb5_data data;
> > @@ -479,11 +479,6 @@ static krb5_error_code ipadb_get_pac(krb5_context
> > kcontext,
> > return KRB5_KDB_DBNOTINITED;
> > }
> >
> > - tmpctx = talloc_new(NULL);
> > - if (!tmpctx) {
> > - return ENOMEM;
> > - }
> > -
> > ied = (struct ipadb_e_data *)client->e_data;
> > if (ied->magic != IPA_E_DATA_MAGIC) {
> > return EINVAL;
> > @@ -493,6 +488,11 @@ static krb5_error_code ipadb_get_pac(krb5_context
> > kcontext,
> > return 0;
> > }
> >
> > + tmpctx = talloc_new(NULL);
> > + if (!tmpctx) {
> > + return ENOMEM;
> > + }
> > +
> > memset(&pac_info, 0, sizeof(pac_info));
> > pac_info.logon_info.info = talloc_zero(tmpctx, struct PAC_LOGON_INFO);
> > if (!tmpctx) {
> Here is an issue -- you are allocating off tmpctx which is not empty
> here (we checked it right above) but then you are checking tmpctx
> rather than pac_info.logon_info.info.
>
> It is an older error but needs to be fixed as well.
Ok added this fix.
> Also please name the patch file according to
> https://fedorahosted.org/freeipa/wiki/PatchFormat :)
I lost count of how many patches I handled, and to be honest I think
this naming convention sucks a bit.
I would be ok changing the number to match a ticket number perhaps, when
there is a ticket attached to it, but for patches like this one all you
really-need it to append a -2 at the end if a new revision is necessary.
Putting arbitrary numbers and names, doesn't make it any easier to
handle to me.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
>From 31e01a3350dfc9f2354c9738f29de23450dac744 Mon Sep 17 00:00:00 2001
From: Simo Sorce <sso...@redhat.com>
Date: Sun, 20 Nov 2011 20:50:11 -0500
Subject: [PATCH] ipa-kdb: fix memleaks in ipa_kdb_mspac.c
---
daemons/ipa-kdb/ipa_kdb_mspac.c | 15 ++++++++-------
1 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index cce1ca9060f0e03d525bb87d843bdd5811e9d20b..b435efeec0215d50694eeb668c0286b80056016f 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -466,7 +466,7 @@ static krb5_error_code ipadb_get_pac(krb5_context kcontext,
TALLOC_CTX *tmpctx;
struct ipadb_e_data *ied;
struct ipadb_context *ipactx;
- LDAPMessage *results;
+ LDAPMessage *results = NULL;
LDAPMessage *lentry;
DATA_BLOB pac_data;
krb5_data data;
@@ -479,11 +479,6 @@ static krb5_error_code ipadb_get_pac(krb5_context kcontext,
return KRB5_KDB_DBNOTINITED;
}
- tmpctx = talloc_new(NULL);
- if (!tmpctx) {
- return ENOMEM;
- }
-
ied = (struct ipadb_e_data *)client->e_data;
if (ied->magic != IPA_E_DATA_MAGIC) {
return EINVAL;
@@ -493,9 +488,14 @@ static krb5_error_code ipadb_get_pac(krb5_context kcontext,
return 0;
}
+ tmpctx = talloc_new(NULL);
+ if (!tmpctx) {
+ return ENOMEM;
+ }
+
memset(&pac_info, 0, sizeof(pac_info));
pac_info.logon_info.info = talloc_zero(tmpctx, struct PAC_LOGON_INFO);
- if (!tmpctx) {
+ if (!pac_info.logon_info.info) {
kerr = ENOMEM;
goto done;
}
@@ -542,6 +542,7 @@ static krb5_error_code ipadb_get_pac(krb5_context kcontext,
kerr = krb5_pac_add_buffer(kcontext, *pac, KRB5_PAC_LOGON_INFO, &data);
done:
+ ldap_msgfree(results);
talloc_free(tmpctx);
return kerr;
}
--
1.7.7.1
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
