Patch 2/9: finish realm should free keys with 0 length too

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Patch 2/9: finish realm should free keys with 0 length too

Alexandr Nedvedicky
Hello,

I'm upgrading kerberos bundled with Solaris to krb5-1.16. Solaris currently
ships krb5-1.15.1. I've noticed there are some memory leaks, while running test
suite, which comes with krb-1.16 (e.g. running 'make check').  I don't think
those memory leaks are critical, though as kerberos newbie I can't be sure, so
I think I'm better to share my findings. All memory leaks were found using
'libumem', which can be found on Solaris (or its OSS sibbling illumos).
All patches are against krb5-1.16 release.

Patch below is kind of guess as I suspect the change might be rather
fixing symptoms than true root cause. I'm still getting familiar with
kerberos soruce code. The leaked memory got always allocated by
stack as follows:
            ADDR          BUFADDR        TIMESTAMP           THREAD
                            CACHE          LASTLOG         CONTENTS
         83ff108          83faa60     cea2131e2b05                1
                          83e2a08          83d31f8                0
                 libumem.so.1`umem_cache_alloc_debug+0x16e
                 libumem.so.1`umem_cache_alloc+0x21f
                 libumem.so.1`umem_alloc+0x85
                 libumem.so.1`malloc+0x2a
                 libumem.so.1`calloc+0x68
                 libkdb5.so.9.0`k5calloc+0x61
                 libkdb5.so.9.0`k5alloc+0x2d
                 libkdb5.so.9.0`k5memdup+0x27
                 libkdb5.so.9.0`krb5_db_fetch_mkey+0x2dd
                 init_realm+0x9a7
                 initialize_realms+0xe8e
                 main+0x1fd
                 _start+0x72

after walking through KDC source code I could spot only one possible culprit
found in patch below. After applying the patch the leak is gone.

regards
sasha

--------8<---------------8<---------------8<------------------8<--------
diff --git a/src/kdc/main.c b/src/kdc/main.c
index 6a78ec596..2f5daed8b 100644
--- a/src/kdc/main.c
+++ b/src/kdc/main.c
@@ -161,7 +161,7 @@ finish_realm(kdc_realm_t *rdp)
     if (rdp->realm_context) {
         if (rdp->realm_mprinc)
             krb5_free_principal(rdp->realm_context, rdp->realm_mprinc);
-        if (rdp->realm_mkey.length && rdp->realm_mkey.contents) {
+        if (rdp->realm_mkey.contents) {
             /* XXX shouldn't memset be zap for safety? */
             memset(rdp->realm_mkey.contents, 0, rdp->realm_mkey.length);
             free(rdp->realm_mkey.contents);
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev