From ebc7a7f22a7484542ff10746ecb6f82569a5a688 Mon Sep 17 00:00:00 2001
From: Greg Hudson <ghudson@mit.edu>
Date: Mon, 13 Apr 2009 18:36:42 +0000
Subject: [PATCH] Make krb5_mkt_resolve error handling work

Very little is likely to go wrong inside krb5_mkt_resolve (it just
allocates memory and plays with mutexes), but if anything did, the
handling was almost always wrong.  Reorganize the function to handle
errors properly, using a helper create_list_node function to simplify
the task.

ticket: 6454

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@22198 dc483132-0cff-0310-8789-dd5450dbe970
---
 src/lib/krb5/keytab/kt_memory.c | 146 +++++++++++++++++---------------
 1 file changed, 77 insertions(+), 69 deletions(-)

diff --git a/src/lib/krb5/keytab/kt_memory.c b/src/lib/krb5/keytab/kt_memory.c
index 1f7717187..b78e7064c 100644
--- a/src/lib/krb5/keytab/kt_memory.c
+++ b/src/lib/krb5/keytab/kt_memory.c
@@ -193,101 +193,109 @@ void krb5int_mkt_finalize(void) {
 	free(node);
     }
 }
-/*
- * This is an implementation specific resolver.  It returns a keytab 
- * initialized with memory keytab routines.
- */
 
-krb5_error_code KRB5_CALLCONV 
-krb5_mkt_resolve(krb5_context context, const char *name, krb5_keytab *id)
+static krb5_error_code
+create_list_node(const char *name, krb5_mkt_list_node **listp)
 {
-    krb5_mkt_data *data = 0;
     krb5_mkt_list_node *list;
-    krb5_error_code err = 0;
+    krb5_mkt_data *data = NULL;
+    krb5_error_code err;
 
-    /* First determine if a memory keytab of this name already exists */
-    err = KTGLOCK;
-    if (err)
-	return(err);
+    *listp = NULL;
 
-    for (list = krb5int_mkt_list; list; list = list->next)
-    {
-    	if (strcmp(name,KTNAME(list->keytab)) == 0) {
-	    /* Found */
-	    *id = list->keytab;
-	    goto done;
-	}
+    list = calloc(1, sizeof(krb5_mkt_list_node));
+    if (list == NULL) {
+	err = ENOMEM;
+	goto cleanup;
     }
 
-    /* We will now create the new key table with the specified name.
-     * We do not drop the global lock, therefore the name will indeed
-     * be unique when we add it.
-     */
-
-    if ((list = (krb5_mkt_list_node *)malloc(sizeof(krb5_mkt_list_node))) == NULL) {
+    list->keytab = calloc(1, sizeof(struct _krb5_kt));
+    if (list->keytab == NULL) {
 	err = ENOMEM;
-	goto done;
+	goto cleanup;
     }
+    list->keytab->ops = &krb5_mkt_ops;
 
-    if ((list->keytab = (krb5_keytab)malloc(sizeof(struct _krb5_kt))) == NULL) {
-	free(list);
+    data = calloc(1, sizeof(krb5_mkt_data));
+    if (data == NULL) {
 	err = ENOMEM;
-	goto done;	
+	goto cleanup;
     }
+    data->link = NULL;
+    data->refcount = 0;
 
-    list->keytab->ops = &krb5_mkt_ops;
-    if ((data = (krb5_mkt_data *)malloc(sizeof(krb5_mkt_data))) == NULL) {
-	free(list->keytab);
-	free(list);
+    data->name = strdup(name);
+    if (data->name == NULL) {
 	err = ENOMEM;
-	goto done;
+	goto cleanup;
     }
-    data->name = NULL;
 
     err = k5_mutex_init(&data->lock);
-    if (err) {
-	free(data);
-	free(list->keytab);
-	free(list);
-	goto done;
-    }
+    if (err)
+	goto cleanup;
 
-    if ((data->name = strdup(name)) == NULL) {
-	k5_mutex_destroy(&data->lock);
-	free(data);
+    list->keytab->data = data;
+    list->keytab->magic = KV5M_KEYTAB;
+    list->next = NULL;
+    *listp = list;
+    return 0;
+
+cleanup:
+    /* data->lock was initialized last, so no need to destroy. */
+    if (data)
+	free(data->name);
+    free(data);
+    if (list)
 	free(list->keytab);
-	free(list);
-	err = ENOMEM;
-	goto done;
-    }
+    free(list);
+    return err;
+}
 
-    data->link = NULL;
-    data->refcount = 0;
-    list->keytab->data = (krb5_pointer)data;
-    list->keytab->magic = KV5M_KEYTAB;
+/*
+ * This is an implementation specific resolver.  It returns a keytab 
+ * initialized with memory keytab routines.
+ */
 
-    list->next = krb5int_mkt_list;
-    krb5int_mkt_list = list;
+krb5_error_code KRB5_CALLCONV 
+krb5_mkt_resolve(krb5_context context, const char *name, krb5_keytab *id)
+{
+    krb5_mkt_list_node *list;
+    krb5_error_code err = 0;
 
-    *id = list->keytab;
+    *id = NULL;
 
-  done:
-    err = KTLOCK(*id);
-    if (err) {
-	k5_mutex_destroy(&data->lock);
-     	if (data && data->name) 
-		free(data->name);
-	free(data);
-	if (list && list->keytab)
-		free(list->keytab);
-	free(list);
-    } else {
-	KTREFCNT(*id)++;
-	KTUNLOCK(*id);
+    /* First determine if a memory keytab of this name already exists */
+    err = KTGLOCK;
+    if (err)
+	return err;
+
+    for (list = krb5int_mkt_list; list; list = list->next) {
+	if (strcmp(name,KTNAME(list->keytab)) == 0)
+	    break;
     }
 
+    if (!list) {
+	/* We will now create the new key table with the specified name.
+	 * We do not drop the global lock, therefore the name will indeed
+	 * be unique when we add it.
+	 */
+	err = create_list_node(name, &list);
+	if (err)
+	    goto done;
+	list->next = krb5int_mkt_list;
+	krb5int_mkt_list = list;
+    }
+
+    /* Increment the reference count on the keytab we found or created. */
+    err = KTLOCK(list->keytab);
+    if (err)
+	goto done;
+    KTREFCNT(list->keytab)++;
+    KTUNLOCK(list->keytab);
+    *id = list->keytab;
+done:
     KTGUNLOCK;
-    return(err);
+    return err;
 }
 
 
-- 
2.26.2