All known memory leaks caused by unfreed allocated memory or unfreed
LDAP results (which should be also done after unsuccessful searches)
are fixed.

One ipadb_need_retry result check was fixed as this function returns
trust in case of a need for retry and not a zero.

https://fedorahosted.org/freeipa/ticket/3413

---

This patch should be combined with Sumit's patch 0093 which would prevent
kdb5kdc from crashing when it is being terminted.

If you want to run krb5kdc in valgrind, check the command in the ticket which
would let valgrind also show symbols in dlopen'ed ipa-kdb plugin.

Martin
From 3927ce572eef0d7fe6668201cec8d519fe7fa815 Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Tue, 12 Feb 2013 11:59:22 +0100
Subject: [PATCH] ipa-kdb: remove memory leaks

All known memory leaks caused by unfreed allocated memory or unfreed
LDAP results (which should be also done after unsuccessful searches)
are fixed.

One ipadb_need_retry result check was fixed as this function returns
trust in case of a need for retry and not a zero.

https://fedorahosted.org/freeipa/ticket/3413
---
 daemons/ipa-kdb/ipa_kdb.c        |  4 ++++
 daemons/ipa-kdb/ipa_kdb.h        |  2 ++
 daemons/ipa-kdb/ipa_kdb_common.c | 13 +++++++++++--
 daemons/ipa-kdb/ipa_kdb_mspac.c  | 18 +++++++++++++++++-
 4 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb.c b/daemons/ipa-kdb/ipa_kdb.c
index 3527cefa10df67d3f17c730ab4483410c736a44f..55a932abdc3aeaa48892715196834a25f9b2c0e1 100644
--- a/daemons/ipa-kdb/ipa_kdb.c
+++ b/daemons/ipa-kdb/ipa_kdb.c
@@ -40,10 +40,14 @@ static void ipadb_context_free(krb5_context kcontext,
 {
     if (*ctx != NULL) {
         free((*ctx)->uri);
+        free((*ctx)->base);
+        free((*ctx)->realm_base);
         /* ldap free lcontext */
         if ((*ctx)->lcontext) {
             ldap_unbind_ext_s((*ctx)->lcontext, NULL, NULL);
         }
+        free((*ctx)->supp_encs);
+        ipadb_mspac_struct_free(&(*ctx)->mspac);
         krb5_free_default_realm(kcontext, (*ctx)->realm);
         free(*ctx);
         *ctx = NULL;
diff --git a/daemons/ipa-kdb/ipa_kdb.h b/daemons/ipa-kdb/ipa_kdb.h
index beff8b208685a7d561fc096c8e734333437bdb58..f472f02458e040b921b9f3f85bcc36fa954785d5 100644
--- a/daemons/ipa-kdb/ipa_kdb.h
+++ b/daemons/ipa-kdb/ipa_kdb.h
@@ -237,6 +237,8 @@ krb5_error_code ipadb_sign_authdata(krb5_context context,
 
 krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx);
 
+void ipadb_mspac_struct_free(struct ipadb_mspac **mspac);
+
 /* DELEGATION CHECKS */
 
 krb5_error_code ipadb_check_allowed_to_delegate(krb5_context kcontext,
diff --git a/daemons/ipa-kdb/ipa_kdb_common.c b/daemons/ipa-kdb/ipa_kdb_common.c
index e04bae6673ddc97325883708d2bca2805f6ae4fa..956b4b611cbdedbe1a9b0415e9b10f762e46fdae 100644
--- a/daemons/ipa-kdb/ipa_kdb_common.c
+++ b/daemons/ipa-kdb/ipa_kdb_common.c
@@ -172,7 +172,7 @@ krb5_error_code ipadb_simple_search(struct ipadb_context *ipactx,
     /* first test if we need to retry to connect */
     if (ret != 0 &&
         ipadb_need_retry(ipactx, ret)) {
-
+        ldap_msgfree(res);
         ret = ldap_search_ext_s(ipactx->lcontext, basedn, scope,
                                 filter, attrs, 0, NULL, NULL,
                                 &std_timeout, LDAP_NO_LIMIT,
@@ -283,6 +283,7 @@ krb5_error_code ipadb_deref_search(struct ipadb_context *ipactx,
     int times;
     int ret;
     int c, i;
+    bool retry;
 
     for (c = 0; deref_attr_names[c]; c++) {
         /* count */ ;
@@ -315,7 +316,8 @@ krb5_error_code ipadb_deref_search(struct ipadb_context *ipactx,
     /* retry once if connection errors (tot. max. 2 tries) */
     times = 2;
     ret = LDAP_SUCCESS;
-    while (!ipadb_need_retry(ipactx, ret) && times > 0) {
+    retry = true;
+    while (retry) {
         times--;
         ret = ldap_search_ext_s(ipactx->lcontext, base_dn,
                                 scope, filter,
@@ -323,11 +325,18 @@ krb5_error_code ipadb_deref_search(struct ipadb_context *ipactx,
                                 ctrl, NULL,
                                 &std_timeout, LDAP_NO_LIMIT,
                                 res);
+        retry = ipadb_need_retry(ipactx, ret) && times > 0;
+
+        if (retry) {
+            /* Free result before next try */
+            ldap_msgfree(*res);
+        }
     }
 
     kerr = ipadb_simple_ldap_to_kerr(ret);
 
 done:
+    ldap_control_free(ctrl[0]);
     ldap_memfree(derefval.bv_val);
     free(ds);
     return kerr;
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 0780e81cb5507ed590cc9b0646ba5919c0084523..6abd51ec019ef45dd52d317b85dcb9584b49f06f 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -944,6 +944,7 @@ static int map_groups(TALLOC_CTX *memctx, krb5_context kcontext,
             goto done;
         }
 
+        ldap_msgfree(results);
         kerr = ipadb_deref_search(ipactx, basedn, LDAP_SCOPE_ONE, filter,
                                   entry_attrs, deref_search_attrs,
                                   memberof_pac_attrs, &results);
@@ -1636,14 +1637,24 @@ krb5_error_code ipadb_sign_authdata(krb5_context context,
     /* put in signed data */
     ad.magic = KV5M_AUTHDATA;
     ad.ad_type = KRB5_AUTHDATA_WIN2K_PAC;
-    ad.contents = (krb5_octet *)pac_data.data;
+    ad.contents = malloc(pac_data.length);
+    if (ad.contents == NULL) {
+        krb5_free_data_contents(context, &pac_data);
+        ad.length = 0;
+        kerr = ENOMEM;
+        goto done;
+    }
+    memcpy(ad.contents, pac_data.data, pac_data.length);
     ad.length = pac_data.length;
+    krb5_free_data_contents(context, &pac_data);
+
     authdata[0] = &ad;
 
     kerr = krb5_encode_authdata_container(context,
                                           KRB5_AUTHDATA_IF_RELEVANT,
                                           authdata,
                                           signed_auth_data);
+    free(ad.contents);
     if (kerr != 0) {
         goto done;
     }
@@ -1697,7 +1708,9 @@ void ipadb_mspac_struct_free(struct ipadb_mspac **mspac)
             free((*mspac)->trusts[i].sid_blacklist_incoming);
             free((*mspac)->trusts[i].sid_blacklist_outgoing);
         }
+        free((*mspac)->trusts);
     }
+    free(*mspac);
 
     *mspac = NULL;
 }
@@ -2040,14 +2053,17 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx)
             if (ret == 0) {
                 ret = string_to_sid(resstr, &gsid);
                 if (ret) {
+                    free(resstr);
                     kerr = ret;
                     goto done;
                 }
                 ret = sid_split_rid(&gsid, &ipactx->mspac->fallback_rid);
                 if (ret) {
+                    free(resstr);
                     kerr = ret;
                     goto done;
                 }
+                free(resstr);
             }
         }
     }
-- 
1.8.1.2

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to