Assume mutex locking cannot fail

Locking and unlocking a non-recursive mutex is a simple memory
operation and should not fail on any reasonable platform with correct
usage.  A pthread mutex can return EDEADLK on lock or EPERM on unlock,
or EINVAL if the mutex is uninitialized, but all of these conditions
would reflect serious bugs in the calling code.

Change the k5_mutex_lock and k5_mutex_unlock wrappers to return void
and adjust all call sites.  Propagate this change through
k5_cc_mutex_lock and k5_cc_mutex_unlock as well.

9 files changed, 45 insertions, 138 deletions

diff --git a/src/lib/gssapi/generic/util_errmap.c b/src/lib/gssapi/generic/util_errmap.c
index c26ea7b05..628a455d2 100644
--- a/src/lib/gssapi/generic/util_errmap.c
+++ b/src/lib/gssapi/generic/util_errmap.c
@@ -183,13 +183,7 @@ OM_uint32 gssint_mecherrmap_map(OM_uint32 minor, const gss_OID_desc * oid)
 
     me.code = minor;
     me.mech = *oid;
-    err = k5_mutex_lock(&mutex);
-    if (err) {
-#ifdef DEBUG
-        if (f != stderr) fclose(f);
-#endif
-        return 0;
-    }
+    k5_mutex_lock(&mutex);
 
     /* Is this status+oid already mapped?  */
     p = mecherrmap_findright(&m, me);
@@ -254,14 +248,11 @@ int gssint_mecherrmap_get(OM_uint32 minor, gss_OID mech_oid,
                           OM_uint32 *mech_minor)
 {
     const struct mecherror *p;
-    int err;
 
     if (minor == 0) {
         return EINVAL;
     }
-    err = k5_mutex_lock(&mutex);
-    if (err)
-        return err;
+    k5_mutex_lock(&mutex);
     p = mecherrmap_findleft(&m, minor);
     k5_mutex_unlock(&mutex);
     if (!p) {
diff --git a/src/lib/gssapi/krb5/acquire_cred.c b/src/lib/gssapi/krb5/acquire_cred.c
index dbc5a701a..0efcad4fe 100644
--- a/src/lib/gssapi/krb5/acquire_cred.c
+++ b/src/lib/gssapi/krb5/acquire_cred.c
@@ -112,11 +112,7 @@ gss_krb5int_register_acceptor_identity(OM_uint32 *minor_status,
             return GSS_S_FAILURE;
     }
 
-    err = k5_mutex_lock(&gssint_krb5_keytab_lock);
-    if (err) {
-        free(new);
-        return GSS_S_FAILURE;
-    }
+    k5_mutex_lock(&gssint_krb5_keytab_lock);
     old = krb5_gss_keytab;
     krb5_gss_keytab = new;
     k5_mutex_unlock(&gssint_krb5_keytab_lock);
@@ -196,11 +192,7 @@ acquire_accept_cred(krb5_context context,
     if (req_keytab != NULL) {
         code = krb5_kt_dup(context, req_keytab, &kt);
     } else {
-        code = k5_mutex_lock(&gssint_krb5_keytab_lock);
-        if (code) {
-            *minor_status = code;
-            return GSS_S_FAILURE;
-        }
+        k5_mutex_lock(&gssint_krb5_keytab_lock);
         if (krb5_gss_keytab != NULL) {
             code = krb5_kt_resolve(context, krb5_gss_keytab, &kt);
             k5_mutex_unlock(&gssint_krb5_keytab_lock);
diff --git a/src/lib/gssapi/krb5/copy_ccache.c b/src/lib/gssapi/krb5/copy_ccache.c
index fbb7a48a1..f3d766613 100644
--- a/src/lib/gssapi/krb5/copy_ccache.c
+++ b/src/lib/gssapi/krb5/copy_ccache.c
@@ -23,11 +23,7 @@ gss_krb5int_copy_ccache(OM_uint32 *minor_status,
 
     /* cred handle will have been validated by gssspi_set_cred_option() */
     k5creds = (krb5_gss_cred_id_t) *cred_handle;
-    code = k5_mutex_lock(&k5creds->lock);
-    if (code) {
-        *minor_status = code;
-        return GSS_S_FAILURE;
-    }
+    k5_mutex_lock(&k5creds->lock);
     if (k5creds->usage == GSS_C_ACCEPT) {
         k5_mutex_unlock(&k5creds->lock);
         *minor_status = (OM_uint32) G_BAD_USAGE;
diff --git a/src/lib/gssapi/krb5/init_sec_context.c b/src/lib/gssapi/krb5/init_sec_context.c
index d4c987a09..1bc69ca31 100644
--- a/src/lib/gssapi/krb5/init_sec_context.c
+++ b/src/lib/gssapi/krb5/init_sec_context.c
@@ -1014,9 +1014,7 @@ krb5_gss_init_context (krb5_context *ctxp)
     if (err)
         return err;
 #ifndef _WIN32
-    err = k5_mutex_lock(&kg_kdc_flag_mutex);
-    if (err)
-        return err;
+    k5_mutex_lock(&kg_kdc_flag_mutex);
     is_kdc = kdc_flag;
     k5_mutex_unlock(&kg_kdc_flag_mutex);
 
@@ -1041,10 +1039,7 @@ krb5int_gss_use_kdc_context(OM_uint32 *minor_status,
     err = gss_krb5int_initialize_library();
     if (err)
         return err;
-    *minor_status = k5_mutex_lock(&kg_kdc_flag_mutex);
-    if (*minor_status) {
-        return GSS_S_FAILURE;
-    }
+    k5_mutex_lock(&kg_kdc_flag_mutex);
     kdc_flag = 1;
     k5_mutex_unlock(&kg_kdc_flag_mutex);
     return GSS_S_COMPLETE;
diff --git a/src/lib/gssapi/krb5/naming_exts.c b/src/lib/gssapi/krb5/naming_exts.c
index f44f0d2cf..10dbe321a 100644
--- a/src/lib/gssapi/krb5/naming_exts.c
+++ b/src/lib/gssapi/krb5/naming_exts.c
@@ -119,15 +119,10 @@ kg_duplicate_name(krb5_context context,
 {
     krb5_error_code code;
 
-    code = k5_mutex_lock(&src->lock);
-    if (code != 0)
-        return code;
-
+    k5_mutex_lock(&src->lock);
     code = kg_init_name(context, src->princ, src->service, src->host,
                         src->ad_context, 0, dst);
-
     k5_mutex_unlock(&src->lock);
-
     return code;
 }
 
@@ -282,11 +277,7 @@ krb5_gss_inquire_name(OM_uint32 *minor_status,
 
     kname = (krb5_gss_name_t)name;
 
-    code = k5_mutex_lock(&kname->lock);
-    if (code != 0) {
-        *minor_status = code;
-        return GSS_S_FAILURE;
-    }
+    k5_mutex_lock(&kname->lock);
 
     if (kname->ad_context == NULL) {
         code = krb5_authdata_context_init(context, &kname->ad_context);
@@ -343,13 +334,7 @@ krb5_gss_get_name_attribute(OM_uint32 *minor_status,
     }
 
     kname = (krb5_gss_name_t)name;
-
-    code = k5_mutex_lock(&kname->lock);
-    if (code != 0) {
-        *minor_status = code;
-        krb5_free_context(context);
-        return GSS_S_FAILURE;
-    }
+    k5_mutex_lock(&kname->lock);
 
     if (kname->ad_context == NULL) {
         code = krb5_authdata_context_init(context, &kname->ad_context);
@@ -421,12 +406,7 @@ krb5_gss_set_name_attribute(OM_uint32 *minor_status,
     }
 
     kname = (krb5_gss_name_t)name;
-
-    code = k5_mutex_lock(&kname->lock);
-    if (code != 0) {
-        *minor_status = code;
-        return GSS_S_FAILURE;
-    }
+    k5_mutex_lock(&kname->lock);
 
     if (kname->ad_context == NULL) {
         code = krb5_authdata_context_init(context, &kname->ad_context);
@@ -476,12 +456,7 @@ krb5_gss_delete_name_attribute(OM_uint32 *minor_status,
     }
 
     kname = (krb5_gss_name_t)name;
-
-    code = k5_mutex_lock(&kname->lock);
-    if (code != 0) {
-        *minor_status = code;
-        return GSS_S_FAILURE;
-    }
+    k5_mutex_lock(&kname->lock);
 
     if (kname->ad_context == NULL) {
         code = krb5_authdata_context_init(context, &kname->ad_context);
@@ -528,12 +503,7 @@ krb5_gss_map_name_to_any(OM_uint32 *minor_status,
     }
 
     kname = (krb5_gss_name_t)name;
-
-    code = k5_mutex_lock(&kname->lock);
-    if (code != 0) {
-        *minor_status = code;
-        return GSS_S_FAILURE;
-    }
+    k5_mutex_lock(&kname->lock);
 
     if (kname->ad_context == NULL) {
         code = krb5_authdata_context_init(context, &kname->ad_context);
@@ -585,12 +555,7 @@ krb5_gss_release_any_name_mapping(OM_uint32 *minor_status,
     }
 
     kname = (krb5_gss_name_t)name;
-
-    code = k5_mutex_lock(&kname->lock);
-    if (code != 0) {
-        *minor_status = code;
-        return GSS_S_FAILURE;
-    }
+    k5_mutex_lock(&kname->lock);
 
     if (kname->ad_context == NULL) {
         code = krb5_authdata_context_init(context, &kname->ad_context);
@@ -646,12 +611,7 @@ krb5_gss_export_name_composite(OM_uint32 *minor_status,
     }
 
     kname = (krb5_gss_name_t)name;
-
-    code = k5_mutex_lock(&kname->lock);
-    if (code != 0) {
-        *minor_status = code;
-        return GSS_S_FAILURE;
-    }
+    k5_mutex_lock(&kname->lock);
 
     code = krb5_unparse_name(context, kname->princ, &princstr);
     if (code != 0)
diff --git a/src/lib/gssapi/krb5/s4u_gss_glue.c b/src/lib/gssapi/krb5/s4u_gss_glue.c
index a7e720b80..4381a8442 100644
--- a/src/lib/gssapi/krb5/s4u_gss_glue.c
+++ b/src/lib/gssapi/krb5/s4u_gss_glue.c
@@ -58,11 +58,7 @@ kg_impersonate_name(OM_uint32 *minor_status,
     if (impersonator_cred->req_enctypes != NULL)
         in_creds.keyblock.enctype = impersonator_cred->req_enctypes[0];
 
-    code = k5_mutex_lock(&user->lock);
-    if (code != 0) {
-        *minor_status = code;
-        return GSS_S_FAILURE;
-    }
+    k5_mutex_lock(&user->lock);
 
     if (user->ad_context != NULL) {
         code = krb5_authdata_export_authdata(context,
diff --git a/src/lib/gssapi/krb5/set_allowable_enctypes.c b/src/lib/gssapi/krb5/set_allowable_enctypes.c
index 7ef4aeedb..d9fd279ed 100644
--- a/src/lib/gssapi/krb5/set_allowable_enctypes.c
+++ b/src/lib/gssapi/krb5/set_allowable_enctypes.c
@@ -91,9 +91,7 @@ gss_krb5int_set_allowable_enctypes(OM_uint32 *minor_status,
             }
         }
     } else {
-        kerr = k5_mutex_lock(&cred->lock);
-        if (kerr)
-            goto error_out;
+        k5_mutex_lock(&cred->lock);
         if (cred->req_enctypes)
             free(cred->req_enctypes);
         cred->req_enctypes = NULL;
@@ -110,11 +108,7 @@ gss_krb5int_set_allowable_enctypes(OM_uint32 *minor_status,
         kerr = ENOMEM;
         goto error_out;
     }
-    kerr = k5_mutex_lock(&cred->lock);
-    if (kerr) {
-        free(new_ktypes);
-        goto error_out;
-    }
+    k5_mutex_lock(&cred->lock);
     if (cred->req_enctypes)
         free(cred->req_enctypes);
     cred->req_enctypes = new_ktypes;
diff --git a/src/lib/gssapi/krb5/val_cred.c b/src/lib/gssapi/krb5/val_cred.c
index 234cf69c0..cb1cb9393 100644
--- a/src/lib/gssapi/krb5/val_cred.c
+++ b/src/lib/gssapi/krb5/val_cred.c
@@ -37,12 +37,7 @@ krb5_gss_validate_cred_1(OM_uint32 *minor_status, gss_cred_id_t cred_handle,
     krb5_principal princ;
 
     cred = (krb5_gss_cred_id_t) cred_handle;
-
-    code = k5_mutex_lock(&cred->lock);
-    if (code) {
-        *minor_status = code;
-        return GSS_S_FAILURE;
-    }
+    k5_mutex_lock(&cred->lock);
 
     if (cred->ccache && cred->expire != 0) {
         if ((code = krb5_cc_get_principal(context, cred->ccache, &princ))) {
diff --git a/src/lib/gssapi/mechglue/g_initialize.c b/src/lib/gssapi/mechglue/g_initialize.c
index b801e122a..f5b8b1588 100644
--- a/src/lib/gssapi/mechglue/g_initialize.c
+++ b/src/lib/gssapi/mechglue/g_initialize.c
@@ -171,9 +171,7 @@ gss_OID *oid;
 	if (*minor_status != 0)
 		return (GSS_S_FAILURE);
 
-	*minor_status = k5_mutex_lock(&g_mechListLock);
-	if (*minor_status)
-		return GSS_S_FAILURE;
+	k5_mutex_lock(&g_mechListLock);
 	aMech = g_mechList;
 	while (aMech != NULL) {
 
@@ -252,13 +250,9 @@ gss_OID_set *mechSet_out;
 	 * need to lock the g_mechSet in case someone tries to update it while
 	 * I'm copying it.
 	 */
-	*minorStatus = k5_mutex_lock(&g_mechSetLock);
-	if (*minorStatus) {
-		return GSS_S_FAILURE;
-	}
-
+	k5_mutex_lock(&g_mechSetLock);
 	status = generic_gss_copy_oid_set(minorStatus, &g_mechSet, mechSet_out);
-	(void) k5_mutex_unlock(&g_mechSetLock);
+	k5_mutex_unlock(&g_mechSetLock);
 	return (status);
 } /* gss_indicate_mechs */
 
@@ -293,8 +287,7 @@ build_mechSet(void)
 	 * since we are accessing parts of the mechList which could be
 	 * modified.
 	 */
-	if (k5_mutex_lock(&g_mechListLock) != 0)
-		return GSS_S_FAILURE;
+	k5_mutex_lock(&g_mechListLock);
 
 #if 0
 	/*
@@ -316,8 +309,7 @@ build_mechSet(void)
 	 * we need to lock the mech set so that no one else will
 	 * try to read it as we are re-creating it
 	 */
-	if (k5_mutex_lock(&g_mechSetLock) != 0)
-		return GSS_S_FAILURE;
+	k5_mutex_lock(&g_mechSetLock);
 
 	/* if the oid list already exists we must free it first */
 	free_mechSet();
@@ -335,8 +327,8 @@ build_mechSet(void)
 		g_mechSet.elements =
 			(gss_OID) calloc(count, sizeof (gss_OID_desc));
 		if (g_mechSet.elements == NULL) {
-			(void) k5_mutex_unlock(&g_mechSetLock);
-			(void) k5_mutex_unlock(&g_mechListLock);
+			k5_mutex_unlock(&g_mechSetLock);
+			k5_mutex_unlock(&g_mechListLock);
 			return (GSS_S_FAILURE);
 		}
 
@@ -364,8 +356,8 @@ build_mechSet(void)
 				free(g_mechSet.elements);
 				g_mechSet.count = 0;
 				g_mechSet.elements = NULL;
-				(void) k5_mutex_unlock(&g_mechSetLock);
-				(void) k5_mutex_unlock(&g_mechListLock);
+				k5_mutex_unlock(&g_mechSetLock);
+				k5_mutex_unlock(&g_mechListLock);
 				return (GSS_S_FAILURE);
 			}
 			g_OID_copy(curItem, mList->mech_type);
@@ -377,8 +369,8 @@ build_mechSet(void)
 #if 0
 	g_mechSetTime = fileInfo.st_mtime;
 #endif
-	(void) k5_mutex_unlock(&g_mechSetLock);
-	(void) k5_mutex_unlock(&g_mechListLock);
+	k5_mutex_unlock(&g_mechSetLock);
+	k5_mutex_unlock(&g_mechListLock);
 
 	return GSS_S_COMPLETE;
 }
@@ -402,19 +394,18 @@ const gss_OID oid;
 		return (NULL);
 
 	/* make sure we have fresh data */
-	if (k5_mutex_lock(&g_mechListLock) != 0)
-		return NULL;
+	k5_mutex_lock(&g_mechListLock);
 	updateMechList();
 
 	if ((aMech = searchMechList(oid)) == NULL ||
 		aMech->optionStr == NULL) {
-		(void) k5_mutex_unlock(&g_mechListLock);
+		k5_mutex_unlock(&g_mechListLock);
 		return (NULL);
 	}
 
 	if (aMech->optionStr)
 		modOptions = strdup(aMech->optionStr);
-	(void) k5_mutex_unlock(&g_mechListLock);
+	k5_mutex_unlock(&g_mechListLock);
 
 	return (modOptions);
 } /* gssint_get_modOptions */
@@ -924,8 +915,7 @@ gssint_select_mech_type(OM_uint32 *minor, gss_const_OID oid,
 	if (gssint_mechglue_initialize_library() != 0)
 		return GSS_S_FAILURE;
 
-	if (k5_mutex_lock(&g_mechListLock) != 0)
-		return GSS_S_FAILURE;
+	k5_mutex_lock(&g_mechListLock);
 
 	/* Read conf file at least once so that interposer plugins have a
 	 * chance of getting initialized. */
@@ -953,7 +943,7 @@ gssint_select_mech_type(OM_uint32 *minor, gss_const_OID oid,
 	status = GSS_S_BAD_MECH;
 
 done:
-	(void)k5_mutex_unlock(&g_mechListLock);
+	k5_mutex_unlock(&g_mechListLock);
 	return status;
 }
 
@@ -972,8 +962,7 @@ gssint_get_public_oid(gss_const_OID oid)
 	if (gssint_mechglue_initialize_library() != 0)
 		return GSS_C_NO_OID;
 
-	if (k5_mutex_lock(&g_mechListLock) != 0)
-		return GSS_C_NO_OID;
+	k5_mutex_lock(&g_mechListLock);
 
 	for (minfo = g_mechList; minfo != NULL; minfo = minfo->next) {
 		if (minfo->is_interposer)
@@ -986,7 +975,7 @@ gssint_get_public_oid(gss_const_OID oid)
 		}
 	}
 
-	(void)k5_mutex_unlock(&g_mechListLock);
+	k5_mutex_unlock(&g_mechListLock);
 	return public_oid;
 }
 
@@ -1045,8 +1034,7 @@ gssint_get_mechanism(gss_const_OID oid)
 	if (gssint_mechglue_initialize_library() != 0)
 		return (NULL);
 
-	if (k5_mutex_lock(&g_mechListLock) != 0)
-		return NULL;
+	k5_mutex_lock(&g_mechListLock);
 
 	/* Check if the mechanism is already loaded. */
 	aMech = g_mechList;
@@ -1054,11 +1042,11 @@ gssint_get_mechanism(gss_const_OID oid)
 		oid = aMech->mech_type;
 	while (aMech != NULL) {
 		if (g_OID_equal(aMech->mech_type, oid) && aMech->mech) {
-			(void)k5_mutex_unlock(&g_mechListLock);
+			k5_mutex_unlock(&g_mechListLock);
 			return aMech->mech;
 		} else if (aMech->int_mech_type != GSS_C_NO_OID &&
 			   g_OID_equal(aMech->int_mech_type, oid)) {
-			(void)k5_mutex_unlock(&g_mechListLock);
+			k5_mutex_unlock(&g_mechListLock);
 			return aMech->int_mech;
 		}
 		aMech = aMech->next;
@@ -1074,13 +1062,13 @@ gssint_get_mechanism(gss_const_OID oid)
 
 	/* is the mechanism present in the list ? */
 	if (aMech == NULL) {
-		(void) k5_mutex_unlock(&g_mechListLock);
+		k5_mutex_unlock(&g_mechListLock);
 		return ((gss_mechanism)NULL);
 	}
 
 	/* has another thread loaded the mech */
 	if (aMech->mech) {
-		(void) k5_mutex_unlock(&g_mechListLock);
+		k5_mutex_unlock(&g_mechListLock);
 		return (aMech->mech);
 	}
 
@@ -1092,7 +1080,7 @@ gssint_get_mechanism(gss_const_OID oid)
 		(void) syslog(LOG_INFO, "libgss dlopen(%s): %s\n",
 				aMech->uLibName, dlerror());
 #endif
-		(void) k5_mutex_unlock(&g_mechListLock);
+		k5_mutex_unlock(&g_mechListLock);
 		return ((gss_mechanism)NULL);
 	}
 
@@ -1111,13 +1099,13 @@ gssint_get_mechanism(gss_const_OID oid)
 		(void) syslog(LOG_INFO, "unable to initialize mechanism"
 				" library [%s]\n", aMech->uLibName);
 #endif
-		(void) k5_mutex_unlock(&g_mechListLock);
+		k5_mutex_unlock(&g_mechListLock);
 		return ((gss_mechanism)NULL);
 	}
 
 	aMech->dl_handle = dl;
 
-	(void) k5_mutex_unlock(&g_mechListLock);
+	k5_mutex_unlock(&g_mechListLock);
 	return (aMech->mech);
 } /* gssint_get_mechanism */