Discussion:
Patch 1/9: pkinit should free realm identity certificates
Alexandr Nedvedicky
2018-02-20 00:47:07 UTC
Permalink
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.

The first patch fixes tiny memory leak in KDC. KDC does not seem to attempt to
release identity credentials on exit at all.

regards
sasha

--------8<---------------8<---------------8<------------------8<--------

diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
index dbe0acbdf..d3b4ad5d8 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
@@ -952,9 +952,32 @@ pkinit_init_certs(pkinit_identity_crypto_context ctx)
return retval;
}

+static void
+pkinit_free_cred(pkinit_cred_info creds)
+{
+ if (creds != NULL) {
+ if (creds->cert != NULL) {
+ X509_free(creds->cert);
+ creds->cert = NULL;
+ }
+
+ if (creds->key != NULL) {
+ EVP_PKEY_free(creds->key);
+ creds->key = NULL;
+ }
+
+ free(creds->name);
+ creds->name = NULL;
+
+ free(creds);
+ }
+}
+
static void
pkinit_fini_certs(pkinit_identity_crypto_context ctx)
{
+ unsigned int i;
+
if (ctx == NULL)
return;

@@ -972,6 +995,11 @@ pkinit_fini_certs(pkinit_identity_crypto_context ctx)

if (ctx->revoked != NULL)
sk_X509_CRL_pop_free(ctx->revoked, X509_CRL_free);
+
+ for (i = 0; i < MAX_CREDS_ALLOWED; i++) {
+ pkinit_free_cred(ctx->creds[i]);
+ ctx->creds[i] = NULL;
+ }
}

static krb5_error_code
_______________________________________________
krbdev mailing list ***@mit.edu
https://mailman.mit.edu/mailman/listinfo/krbdev
Greg Hudson
2018-02-20 16:25:58 UTC
Permalink
On 02/19/2018 07:47 PM, Alexandr Nedvedicky wrote:> I'm upgrading
kerberos bundled with Solaris to krb5-1.16. Solaris currently
Post by Alexandr Nedvedicky
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.
Thanks for these. I will open one or more pull requests at
https://github.com/krb5/krb5 (unless you would prefer to do that
yourself) and review them there.

_______________________________________________
krbdev mailing list ***@mit.edu
https://mailman.mit.edu/mailman/listinfo/krbdev
Alexandr Nedvedicky
2018-02-20 16:39:02 UTC
Permalink
Post by Greg Hudson
On 02/19/2018 07:47 PM, Alexandr Nedvedicky wrote:> I'm upgrading
kerberos bundled with Solaris to krb5-1.16. Solaris currently
Post by Alexandr Nedvedicky
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.
Thanks for these. I will open one or more pull requests at
https://github.com/krb5/krb5 (unless you would prefer to do that
yourself) and review them there.
O.K. I'll try to open those pull request, just to get the chance to 'learn
the github'

will do it later today or tomorrow morning my time (Europe)

regards
sasha
_______________________________________________
krbdev mailing list ***@mit.edu
https://mailman.mit.edu/mailman/listinfo/krbdev

Loading...