Avoid calling krb5_free_context(NULL)

Several places in the code currently call krb5_free_context(ctx) in a
cleanup code path, where 'ctx' may or may not be NULL. This is not
guaranteed to be okay, so check for NULL to make sure we don't cause
issues in these code paths.

While we are here cleaning up krb5_free_context() calls, also fix a
few call sites in afscp_util.c that were not calling krb5_free_context
in all error paths.

Change-Id: I881f01bdf94f00079f84c4bd4bcfa58998e51ac9
Reviewed-on: https://gerrit.openafs.org/13461
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

diff --git a/src/aklog/aklog.c b/src/aklog/aklog.c
index 63b5a7f..7d190bd 100644 (file)
--- a/src/aklog/aklog.c
+++ b/src/aklog/aklog.c
@@ -317,9 +317,10 @@ redirect_errors(const char *who, afs_int32 code, const char *fmt, va_list ap)
            krb5_svc_get_msg(code,&str);
 #elif defined(HAVE_KRB5_GET_ERROR_MESSAGE)
            krb5_context context;
-           krb5_init_context(&context);
-           str = krb5_get_error_message(context, code);
-           krb5_free_context(context);
+           if (krb5_init_context(&context) == 0) {
+                str = krb5_get_error_message(context, code);
+                krb5_free_context(context);
+            }
 #else
            ; /* IRIX apparently has neither: use the string we have */
 #endif

diff --git a/src/libafscp/afscp_util.c b/src/libafscp/afscp_util.c
index 47fcfcd..0e5a0e2 100644 (file)
--- a/src/libafscp/afscp_util.c
+++ b/src/libafscp/afscp_util.c
@@ -216,7 +216,7 @@ _GetSecurityObject(struct afscp_cell *cell)
 {
     int code = ENOENT;
 #ifdef HAVE_KERBEROS
-    krb5_context context;
+    krb5_context context = NULL;
     krb5_creds match;
     krb5_creds *cred;
     krb5_ccache cc;
@@ -286,7 +286,6 @@ _GetSecurityObject(struct afscp_cell *cell)
        krb5_free_cred_contents(context, &match);
        if (cc)
            krb5_cc_close(context, cc);
-       krb5_free_context(context);
        goto try_anon;
     }
 
@@ -303,7 +302,6 @@ _GetSecurityObject(struct afscp_cell *cell)
            krb5_free_cred_contents(context, &match);
            if (cc)
                krb5_cc_close(context, cc);
-           krb5_free_context(context);
            goto try_anon;
        }
     }
@@ -325,7 +323,10 @@ _GetSecurityObject(struct afscp_cell *cell)
     cell->scindex = 2;
     return 0;
 
-    try_anon:
+ try_anon:
+    if (context != NULL) {
+        krb5_free_context(context);
+    }
 #endif /* HAVE_KERBEROS */
     if (try_anonymous)
        return _GetNullSecurityObject(cell);

diff --git a/src/rxgk/rxgk_crypto_rfc3961.c b/src/rxgk/rxgk_crypto_rfc3961.c
index 5485ed5..df8745a 100644 (file)
--- a/src/rxgk/rxgk_crypto_rfc3961.c
+++ b/src/rxgk/rxgk_crypto_rfc3961.c
@@ -232,7 +232,9 @@ rxgk_make_key(rxgk_key *key_out, void *raw_key, afs_uint32 length,
     *key_out = keyblock2key(new_key);
  done:
     if (ret != 0 && new_key != NULL) {
-       krb5_free_context(new_key->init_ctx);
+        if (new_key->init_ctx != NULL) {
+            krb5_free_context(new_key->init_ctx);
+        }
        rxi_Free(new_key, sizeof(*new_key));
     }
     return ktor(ret);
@@ -311,7 +313,9 @@ rxgk_release_key(rxgk_key *key)
     keyblock = key2keyblock(*key);
 
     krb5_free_keyblock_contents(keyblock->init_ctx, &keyblock->key);
-    krb5_free_context(keyblock->init_ctx);
+    if (keyblock->init_ctx != NULL) {
+        krb5_free_context(keyblock->init_ctx);
+    }
     rxi_Free(keyblock, sizeof(*keyblock));
     *key = NULL;
 }
@@ -351,7 +355,9 @@ rxgk_mic_length(rxgk_key key, size_t *out)
     *out = len;
 
  done:
-    krb5_free_context(ctx);
+    if (ctx != NULL) {
+        krb5_free_context(ctx);
+    }
     return ktor(ret);
 }
 
@@ -419,7 +425,9 @@ rxgk_mic_in_key(rxgk_key key, afs_int32 usage, RXGK_Data *in,
     free_Checksum(&cksum);
     if (crypto != NULL)
        krb5_crypto_destroy(ctx, crypto);
-    krb5_free_context(ctx);
+    if (ctx != NULL) {
+        krb5_free_context(ctx);
+    }
     return ktor(ret);
 }
 
@@ -474,7 +482,9 @@ rxgk_check_mic_in_key(rxgk_key key, afs_int32 usage, RXGK_Data *in,
     free_Checksum(&cksum);
     if (crypto != NULL)
        krb5_crypto_destroy(ctx, crypto);
-    krb5_free_context(ctx);
+    if (ctx != NULL) {
+        krb5_free_context(ctx);
+    }
     return ktor(ret);
 }
 
@@ -526,7 +536,9 @@ rxgk_encrypt_in_key(rxgk_key key, afs_int32 usage, RXGK_Data *in,
     if (crypto != NULL)
        krb5_crypto_destroy(ctx, crypto);
     krb5_data_free(&kd_out);
-    krb5_free_context(ctx);
+    if (ctx != NULL) {
+        krb5_free_context(ctx);
+    }
     return ktor(ret);
 }
 
@@ -580,7 +592,9 @@ rxgk_decrypt_in_key(rxgk_key key, afs_int32 usage, RXGK_Data *in,
     if (crypto != NULL)
        krb5_crypto_destroy(ctx, crypto);
     krb5_data_free(&kd_out);
-    krb5_free_context(ctx);
+    if (ctx != NULL) {
+        krb5_free_context(ctx);
+    }
     return ktor(ret);
 }
 
@@ -646,7 +660,9 @@ PRFplus(krb5_data *out, krb5_enctype enctype, rxgk_key k0,
     if (crypto != NULL)
        krb5_crypto_destroy(ctx, crypto);
     krb5_data_free(&prf_out);
-    krb5_free_context(ctx);
+    if (ctx != NULL) {
+        krb5_free_context(ctx);
+    }
     rxi_Free(prf_in.data, prf_in.length);
     if (pre_key != NULL)
        rxi_Free(pre_key, iterations * block_len);
@@ -762,7 +778,9 @@ rxgk_cipher_expansion(rxgk_key k0, afs_uint32 *len_out)
  done:
     if (crypto != NULL)
        krb5_crypto_destroy(ctx, crypto);
-    krb5_free_context(ctx);
+    if (ctx != NULL) {
+        krb5_free_context(ctx);
+    }
     return ktor(ret);
 }
 

diff --git a/src/rxkad/ticket5.c b/src/rxkad/ticket5.c
index 63ab328..af3ee33 100644 (file)
--- a/src/rxkad/ticket5.c
+++ b/src/rxkad/ticket5.c
@@ -615,7 +615,9 @@ cleanup:
     if (cr != NULL)
        krb5_crypto_destroy(context, cr);
     krb5_free_keyblock_contents(context, &kb);
-    krb5_free_context(context);
+    if (context != NULL) {
+        krb5_free_context(context);
+    }
     rxi_Free(buf, allocsiz);
     if ((code & 0xFFFFFF00) == ERROR_TABLE_BASE_asn1)
        return RXKADINCONSISTENCY;