Close memory leaks in the kadm5 library

Russ Allbery <[email protected]>
Newsgroups gmane.comp.encryption.kerberos.heimdal.general
Organization The Eyrie
Message-ID <[email protected]>
valgrind revealed a bunch of memory leaks in a simple kadmin client (part
of the pam-krb5 test suite, to set a principal's password expired), all of
which traced to two places.

First, if a ticket cache were not passed into kadm5_c_init_with_context,
kadm5_connect would create a temporary ticket cache for the kadmin service
ticket.  This, via get_new_cache, would create a temporary unique memory
cache.  But, on completion of the connection, that cache was only closed,
not destroyed, which left its contents still allocated.  Since this is a
temporary, unique memory cache, there's no reason to leave it allocated
once the function completes, so close it with krb5_cc_destroy instead.

Second, kadm5_c_destroy was not freeing the kadm5_client_context itself,
just its contents.

After the attached patch, a simple kadmin client (and, indeed, the entire
pam-krb5 test suite) run under valgrind with --leak-check=full
--show-reachable=yes does not uncover any memory leaks not covered by the
following suppressions:

{
   heimdal-krb5-init-context-once
   Memcheck:Leak
   fun:*alloc
   ...
   fun:init_context_once
}
{
   heimdal-krb5-reg-plugins-once
   Memcheck:Leak
   fun:*alloc
   ...
   fun:krb5_plugin_register
   fun:reg_def_plugins_once
}
{
   heimdal-krb5-openssl-init
   Memcheck:Leak
   fun:*alloc
   obj:*
   fun:CRYPTO_*alloc
}

-- 
Russ Allbery ([email protected])             <http://www.eyrie.org/~eagle/>
0001-Close-memory-leaks-in-the-client-kadmin-library.patch (text/x-diff, 1.6 KB)
From a8e238e3237ed56b770880848399da8d4b2e9dbb Mon Sep 17 00:00:00 2001
From: Russ Allbery <[email protected]>
Date: Thu, 22 Dec 2011 13:23:14 -0800
Subject: [PATCH] Close memory leaks in the client kadmin library

kadm5_c_destroy was not freeing the kadm5_client_context, just its
contents.  Also free the context itself.

If a ticket cache was not passed into kadm5_c_init_with_context,
kadm5_connect would create a temporary ticket cache for the kadmin
service ticket.  This, via get_new_cache, would create a temporary
unique memory cache.  But, on completion of the connection, that
cache was only closed, not destroyed, which left its contents still
allocated.  Since this is a temporary, unique memory cache, there's
no reason to leave it allocated once the function completes, so
close it with krb5_cc_destroy instead.
---
 lib/kadm5/destroy_c.c |    1 +
 lib/kadm5/init_c.c    |    2 +-
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/lib/kadm5/destroy_c.c b/lib/kadm5/destroy_c.c
index 06a0802..da9da39 100644
--- a/lib/kadm5/destroy_c.c
+++ b/lib/kadm5/destroy_c.c
@@ -51,5 +51,6 @@ kadm5_c_destroy(void *server_handle)
 	krb5_auth_con_free(context->context, context->ac);
     if(context->my_context)
 	krb5_free_context(context->context);
+    free(context);
     return 0;
 }
diff --git a/lib/kadm5/init_c.c b/lib/kadm5/init_c.c
index f21cd32..2a9009a 100644
--- a/lib/kadm5/init_c.c
+++ b/lib/kadm5/init_c.c
@@ -552,7 +552,7 @@ kadm_connect(kadm5_client_context *ctx)
 
     krb5_free_principal(context, server);
     if(ctx->ccache == NULL)
-	krb5_cc_close(context, cc);
+	krb5_cc_destroy(context, cc);
     ctx->sock = s;
 
     return 0;
-- 
1.7.7.3
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.