Skip to content

Commit

Permalink
Assume mutex locking cannot fail
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
greghudson committed May 14, 2013
1 parent 1799f7b commit 6350fd0
Show file tree
Hide file tree
Showing 36 changed files with 296 additions and 736 deletions.
24 changes: 10 additions & 14 deletions src/include/k5-thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -378,17 +378,17 @@ static inline int k5_mutex_finish_init(k5_mutex_t *m)
#define k5_mutex_destroy(M) \
(k5_os_mutex_destroy(M))

#if __GNUC__ >= 4
static int k5_mutex_lock(k5_mutex_t *)
__attribute__((warn_unused_result));
#endif
static inline int k5_mutex_lock(k5_mutex_t *m)
static inline void k5_mutex_lock(k5_mutex_t *m)
{
return k5_os_mutex_lock(m);
int r = k5_os_mutex_lock(m);
assert(r == 0);
}

#define k5_mutex_unlock(M) \
(k5_os_mutex_unlock(M))
static inline void k5_mutex_unlock(k5_mutex_t *m)
{
int r = k5_os_mutex_unlock(m);
assert(r == 0);
}

#define k5_mutex_assert_locked(M) ((void)(M))
#define k5_mutex_assert_unlocked(M) ((void)(M))
Expand Down Expand Up @@ -423,12 +423,8 @@ extern int k5_key_delete(k5_key_t);

extern int KRB5_CALLCONV krb5int_mutex_alloc (k5_mutex_t **);
extern void KRB5_CALLCONV krb5int_mutex_free (k5_mutex_t *);
extern int KRB5_CALLCONV krb5int_mutex_lock (k5_mutex_t *)
#if __GNUC__ >= 4
__attribute__((warn_unused_result))
#endif
;
extern int KRB5_CALLCONV krb5int_mutex_unlock (k5_mutex_t *);
extern void KRB5_CALLCONV krb5int_mutex_lock (k5_mutex_t *);
extern void KRB5_CALLCONV krb5int_mutex_unlock (k5_mutex_t *);

/* In time, many of the definitions above should move into the support
library, and this file should be greatly simplified. For type
Expand Down
9 changes: 2 additions & 7 deletions src/lib/crypto/krb/prng_fortuna.c
Original file line number Diff line number Diff line change
Expand Up @@ -391,9 +391,7 @@ krb5_c_random_add_entropy(krb5_context context, unsigned int randsource,
ret = krb5int_crypto_init();
if (ret)
return ret;
ret = k5_mutex_lock(&fortuna_lock);
if (ret)
return ret;
k5_mutex_lock(&fortuna_lock);
if (randsource == KRB5_C_RANDSOURCE_OSRAND ||
randsource == KRB5_C_RANDSOURCE_TRUSTEDPARTY) {
/* These sources contain enough entropy that we should use them
Expand All @@ -414,17 +412,14 @@ krb5_c_random_add_entropy(krb5_context context, unsigned int randsource,
krb5_error_code KRB5_CALLCONV
krb5_c_random_make_octets(krb5_context context, krb5_data *outdata)
{
krb5_error_code ret;
#ifdef _WIN32
DWORD pid = GetCurrentProcessId();
#else
pid_t pid = getpid();
#endif
unsigned char pidbuf[4];

ret = k5_mutex_lock(&fortuna_lock);
if (ret)
return ret;
k5_mutex_lock(&fortuna_lock);

if (!have_entropy) {
k5_mutex_unlock(&fortuna_lock);
Expand Down
13 changes: 2 additions & 11 deletions src/lib/gssapi/generic/util_errmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
12 changes: 2 additions & 10 deletions src/lib/gssapi/krb5/acquire_cred.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 1 addition & 5 deletions src/lib/gssapi/krb5/copy_ccache.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
9 changes: 2 additions & 7 deletions src/lib/gssapi/krb5/init_sec_context.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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;
Expand Down
56 changes: 8 additions & 48 deletions src/lib/gssapi/krb5/naming_exts.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 1 addition & 5 deletions src/lib/gssapi/krb5/s4u_gss_glue.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 2 additions & 8 deletions src/lib/gssapi/krb5/set_allowable_enctypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
7 changes: 1 addition & 6 deletions src/lib/gssapi/krb5/val_cred.c
Original file line number Diff line number Diff line change
Expand Up @@ -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))) {
Expand Down

0 comments on commit 6350fd0

Please sign in to comment.