krb5 commit: Avoid allocating zero key_data structures

Greg Hudson ghudson at MIT.EDU
Tue Jul 16 10:46:48 EDT 2013


https://github.com/krb5/krb5/commit/d9457b501cbab535e5968dbdf195ca334b9fa555
commit d9457b501cbab535e5968dbdf195ca334b9fa555
Author: Greg Hudson <ghudson at mit.edu>
Date:   Mon Jul 15 12:20:26 2013 -0400

    Avoid allocating zero key_data structures
    
    When we allocate space for an array of key_data structures, make sure
    we allocate at least one, so we don't spuriously fail on platforms
    where malloc(0) returns NULL.  Where we use malloc, use k5calloc
    instead.  Where we use krb5_db_alloc or realloc, just allocate an
    extra entry.

 src/lib/kadm5/srv/svr_principal.c                  |   24 ++++++++++---------
 src/lib/kdb/kdb_convert.c                          |   17 +++++--------
 src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c |    3 +-
 3 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/src/lib/kadm5/srv/svr_principal.c b/src/lib/kadm5/srv/svr_principal.c
index 6c7a2c0..2bb8711 100644
--- a/src/lib/kadm5/srv/svr_principal.c
+++ b/src/lib/kadm5/srv/svr_principal.c
@@ -941,11 +941,10 @@ kadm5_get_principal(void *server_handle, krb5_principal principal,
     if (mask & KADM5_KEY_DATA) {
         entry->n_key_data = kdb->n_key_data;
         if(entry->n_key_data) {
-            entry->key_data = malloc(entry->n_key_data*sizeof(krb5_key_data));
-            if (entry->key_data == NULL) {
-                ret = ENOMEM;
+            entry->key_data = k5calloc(entry->n_key_data,
+                                       sizeof(krb5_key_data), &ret);
+            if (entry->key_data == NULL)
                 goto done;
-            }
         } else
             entry->key_data = NULL;
 
@@ -1070,14 +1069,14 @@ int create_history_entry(krb5_context context,
                          krb5_keyblock *hist_key, int n_key_data,
                          krb5_key_data *key_data, osa_pw_hist_ent *hist)
 {
-    int i, ret;
+    krb5_error_code ret;
     krb5_keyblock key;
     krb5_keysalt salt;
+    int i;
 
-    hist->key_data = (krb5_key_data*)malloc(n_key_data*sizeof(krb5_key_data));
+    hist->key_data = k5calloc(n_key_data, sizeof(krb5_key_data), &ret);
     if (hist->key_data == NULL)
-        return ENOMEM;
-    memset(hist->key_data, 0, n_key_data*sizeof(krb5_key_data));
+        return ret;
 
     for (i = 0; i < n_key_data; i++) {
         ret = krb5_dbe_decrypt_key_data(context, NULL, &key_data[i], &key,
@@ -1999,8 +1998,10 @@ kadm5_setkey_principal_3(void *server_handle,
         old_key_data = NULL;
     }
 
-    kdb->key_data = (krb5_key_data*)krb5_db_alloc(handle->context, NULL, (n_keys+n_old_keys)
-                                                  *sizeof(krb5_key_data));
+    /* Allocate one extra key_data to avoid allocating 0 bytes. */
+    kdb->key_data = krb5_db_alloc(handle->context, NULL,
+                                  (n_keys + n_old_keys + 1) *
+                                  sizeof(krb5_key_data));
     if (kdb->key_data == NULL) {
         ret = ENOMEM;
         goto done;
@@ -2330,8 +2331,9 @@ kadm5_purgekeys(void *server_handle,
     old_keydata = kdb->key_data;
     n_old_keydata = kdb->n_key_data;
     kdb->n_key_data = 0;
+    /* Allocate one extra key_data to avoid allocating 0 bytes. */
     kdb->key_data = krb5_db_alloc(handle->context, NULL,
-                                  n_old_keydata * sizeof(krb5_key_data));
+                                  (n_old_keydata + 1) * sizeof(krb5_key_data));
     if (kdb->key_data == NULL) {
         ret = ENOMEM;
         goto done;
diff --git a/src/lib/kdb/kdb_convert.c b/src/lib/kdb/kdb_convert.c
index f08d423..ee3e984 100644
--- a/src/lib/kdb/kdb_convert.c
+++ b/src/lib/kdb/kdb_convert.c
@@ -589,6 +589,7 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry,
     krb5_error_code ret;
     unsigned int prev_n_keys = 0;
     krb5_boolean is_add;
+    void *newptr;
 
     *entry = NULL;
 
@@ -687,15 +688,12 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry,
             if (is_add)
                 ent->key_data = NULL;
 
-            ent->key_data = (krb5_key_data *)realloc(ent->key_data,
-                                                     (ent->n_key_data *
-                                                      sizeof (krb5_key_data)));
-            /* XXX Memory leak: Old key data in
-               records eliminated by resizing to
-               smaller size.  */
-            if (ent->key_data == NULL)
-                /* XXX Memory leak: old storage.  */
-                return (ENOMEM);
+            /* Allocate one extra key data to avoid allocating zero bytes. */
+            newptr = realloc(ent->key_data, (ent->n_key_data + 1) *
+                             sizeof(krb5_key_data));
+            if (newptr == NULL)
+                return ENOMEM;
+            ent->key_data = newptr;
 
 /* BEGIN CSTYLED */
             for (j = prev_n_keys; j < ent->n_key_data; j++) {
@@ -713,7 +711,6 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry,
                 }
 
                 for (cnt = 0; cnt < kp->key_data_ver; cnt++) {
-                    void *newptr;
                     kp->key_data_type[cnt] =  (krb5_int16)kv->k_enctype.k_enctype_val[cnt];
                     kp->key_data_length[cnt] = (krb5_int16)kv->k_contents.k_contents_val[cnt].utf8str_t_len;
                     newptr = realloc(kp->key_data_contents[cnt],
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
index bcdc1dc..c30599e 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
@@ -1380,7 +1380,8 @@ krb5_decode_krbsecretkey(krb5_context context, krb5_db_entry *entries,
         }
         noofkeys += n_kd;
         tmp = key_data;
-        key_data = realloc (key_data, noofkeys * sizeof (krb5_key_data));
+        /* Allocate an extra key data to avoid allocating zero bytes. */
+        key_data = realloc(key_data, (noofkeys + 1) * sizeof (krb5_key_data));
         if (key_data == NULL) {
             key_data = tmp;
             st = ENOMEM;


More information about the cvs-krb5 mailing list