[OpenAFS-devel] FW: memory leaks in krb524d.... (patch to fix)

[Neulinger, Nathan]

FYI, in case anyone is using krb524d to do AFS stuff and doesn't follow the
krbdev list. This patch is against krb5-current to fix a memory leak in
krb524d. 

-- Nathan

------------------------------------------------------------
Nathan Neulinger                       EMail:  nneul@umr.edu
University of Missouri - Rolla         Phone: (573) 341-4841
Computing Services                       Fax: (573) 341-4216


-----Original Message-----
From: Neulinger, Nathan [mailto:nneul@umr.edu] 
Sent: Friday, November 09, 2001 10:53 AM
To: 'krbdev@mit.edu'
Subject: memory leaks in krb524d.... (patch to fix)


Krb524 leaks memory on each request... In lookup_service_key, the keyblock
is partially copied, and copied incorrected, but the kt entry is never
freed. 

That code never free's the entry that gets retrieved, which is a problem,
cause when it does that memcpy, it's copying the pointer to contents, not
the actual contents. 

Index: krb524d.c
===================================================================
RCS file:
/afs/.umr.edu/software/krb5src/cvsroot/krb5-current/src/krb524/krb524d.c,v
retrieving revision 1.2
diff -u -r1.2 krb524d.c
--- krb524d.c   26 Jun 2001 13:58:23 -0000      1.2
+++ krb524d.c   9 Nov 2001 16:48:49 -0000
@@ -425,6 +425,13 @@
          if ((ret = krb5_kt_get_entry(context, kt, p, kvno, ktype,
&entry)))
               return ret;
          memcpy(key, (char *) &entry.key, sizeof(krb5_keyblock));
+
+         key->contents = (krb5_octet *)malloc(key->length);
+         if ( key->contents )
+               memcpy((char *)key->contents, (char *)entry.key.contents,
+               key->length);
+
+         krb5_kt_free_entry(context, &entry);
          return 0;
      } else if (use_master) {
          return kdc_get_server_key(context, p, key, kvnop, ktype, kvno);


It would be more appropriate to use krb5_copy_keyblock, however, that would
require a lot of other changes throughout krb524d.c. Would y'all be willing
to add a krb5_copy_keyblock equivalent that DID NOT do the allocation? In
that case, it would be a simple change to krb524d.c to make it use the
function instead.

The reason this is a problem is that it leaks 104 bytes per request (at
least with the requests I'm doing locally). That adds up pretty quick if
you're a heavy user of krb524d. 

-- Nathan

------------------------------------------------------------
Nathan Neulinger                       EMail:  nneul@umr.edu
University of Missouri - Rolla         Phone: (573) 341-4841
Computing Services                       Fax: (573) 341-4216