From: Neil Horman Date: Mon, 14 Jul 2025 13:12:17 +0000 (-0400) Subject: reduce lock contention when adding objects to ADDED_OBJ hash table X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=88a1fbb8d1b22a7c54483e50eed9ca77d28ee441;p=thirdparty%2Fopenssl.git reduce lock contention when adding objects to ADDED_OBJ hash table Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/28081) --- diff --git a/crypto/objects/obj_dat.c b/crypto/objects/obj_dat.c index 60b4611f35f..e331b73b500 100644 --- a/crypto/objects/obj_dat.c +++ b/crypto/objects/obj_dat.c @@ -46,7 +46,7 @@ static CRYPTO_RWLOCK *ossl_obj_lock = NULL; static CRYPTO_RWLOCK *ossl_obj_nid_lock = NULL; #endif -static CRYPTO_ONCE ossl_obj_lock_init = CRYPTO_ONCE_STATIC_INIT; +static CRYPTO_ONCE ossl_obj_api_init = CRYPTO_ONCE_STATIC_INIT; static ossl_inline void objs_free_locks(void) { @@ -58,7 +58,7 @@ static ossl_inline void objs_free_locks(void) #endif } -DEFINE_RUN_ONCE_STATIC(obj_lock_initialise) +DEFINE_RUN_ONCE_STATIC(obj_api_initialise) { ossl_obj_lock = CRYPTO_THREAD_lock_new(); if (ossl_obj_lock == NULL) @@ -72,43 +72,40 @@ DEFINE_RUN_ONCE_STATIC(obj_lock_initialise) } #endif added = lh_ADDED_OBJ_new(added_obj_hash, added_obj_cmp); - if (added == NULL) + if (added == NULL) { + objs_free_locks(); return 0; + } return 1; } -static ossl_inline int ossl_init_added_lock(void) +static ossl_inline int ossl_init_added_api(void) { #ifndef OPENSSL_NO_AUTOLOAD_CONFIG /* Make sure we've loaded config before checking for any "added" objects */ OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG, NULL); #endif - return RUN_ONCE(&ossl_obj_lock_init, obj_lock_initialise); + return RUN_ONCE(&ossl_obj_api_init, obj_api_initialise); } -static ossl_inline int ossl_obj_write_lock(int lock) +static ossl_inline int ossl_obj_write_lock(void) { - if (!ossl_init_added_lock()) + if (!ossl_init_added_api()) return 0; - if (!lock) - return 1; return CRYPTO_THREAD_write_lock(ossl_obj_lock); } -static ossl_inline int ossl_obj_read_lock(int lock) +static ossl_inline int ossl_obj_read_lock(void) { - if (!ossl_init_added_lock()) + if (!ossl_init_added_api()) return 0; - if (!lock) - return 1; return CRYPTO_THREAD_read_lock(ossl_obj_lock); } -static ossl_inline void ossl_obj_unlock(int lock) +static ossl_inline void ossl_obj_unlock(void) { - if (lock) - CRYPTO_THREAD_unlock(ossl_obj_lock); + CRYPTO_THREAD_unlock(ossl_obj_lock); } static int sn_cmp(const ASN1_OBJECT *const *a, const unsigned int *b) @@ -228,105 +225,22 @@ void ossl_obj_cleanup_int(void) objs_free_locks(); } -/* - * Requires that the ossl_obj_lock be held - * if TSAN_REQUIRES_LOCKING defined - */ -static int obj_new_nid_unlocked(int num) +int OBJ_new_nid(int num) { static TSAN_QUALIFIER int new_nid = NUM_NID; #ifdef TSAN_REQUIRES_LOCKING int i; + ossl_obj_write_lock(); i = new_nid; new_nid += num; - + ossl_obj_unlock(); return i; #else return tsan_add(&new_nid, num); #endif } -int OBJ_new_nid(int num) -{ -#ifdef TSAN_REQUIRES_LOCKING - int i; - - if (!ossl_obj_write_lock(1)) { - ERR_raise(ERR_LIB_OBJ, ERR_R_UNABLE_TO_GET_WRITE_LOCK); - return NID_undef; - } - - i = obj_new_nid_unlocked(num); - - ossl_obj_unlock(1); - - return i; -#else - return obj_new_nid_unlocked(num); -#endif -} - -static int ossl_obj_add_object(const ASN1_OBJECT *obj, int lock) -{ - ASN1_OBJECT *o = NULL; - ADDED_OBJ *ao[4] = { NULL, NULL, NULL, NULL }, *aop[4]; - int i; - - if ((o = OBJ_dup(obj)) == NULL) - return NID_undef; - if ((ao[ADDED_NID] = OPENSSL_malloc(sizeof(*ao[0]))) == NULL - || (o->length != 0 - && obj->data != NULL - && (ao[ADDED_DATA] = OPENSSL_malloc(sizeof(*ao[0]))) == NULL) - || (o->sn != NULL - && (ao[ADDED_SNAME] = OPENSSL_malloc(sizeof(*ao[0]))) == NULL) - || (o->ln != NULL - && (ao[ADDED_LNAME] = OPENSSL_malloc(sizeof(*ao[0]))) == NULL)) - goto err2; - - if (!ossl_obj_write_lock(lock)) { - ERR_raise(ERR_LIB_OBJ, ERR_R_UNABLE_TO_GET_WRITE_LOCK); - goto err2; - } - - for (i = ADDED_DATA; i <= ADDED_NID; i++) { - if (ao[i] != NULL) { - ao[i]->type = i; - ao[i]->obj = o; - aop[i] = lh_ADDED_OBJ_retrieve(added, ao[i]); - if (aop[i] != NULL) - aop[i]->type = -1; - (void)lh_ADDED_OBJ_insert(added, ao[i]); - if (lh_ADDED_OBJ_error(added)) { - if (aop[i] != NULL) - aop[i]->type = i; - while (i-- > ADDED_DATA) { - lh_ADDED_OBJ_delete(added, ao[i]); - if (aop[i] != NULL) - aop[i]->type = i; - } - ERR_raise(ERR_LIB_OBJ, ERR_R_CRYPTO_LIB); - goto err; - } - } - } - o->flags &= - ~(ASN1_OBJECT_FLAG_DYNAMIC | ASN1_OBJECT_FLAG_DYNAMIC_STRINGS | - ASN1_OBJECT_FLAG_DYNAMIC_DATA); - - ossl_obj_unlock(lock); - return o->nid; - - err: - ossl_obj_unlock(lock); - err2: - for (i = ADDED_DATA; i <= ADDED_NID; i++) - OPENSSL_free(ao[i]); - ASN1_OBJECT_free(o); - return NID_undef; -} - ASN1_OBJECT *OBJ_nid2obj(int n) { ADDED_OBJ ad, *adp = NULL; @@ -339,13 +253,12 @@ ASN1_OBJECT *OBJ_nid2obj(int n) ad.type = ADDED_NID; ad.obj = &ob; ob.nid = n; - if (!ossl_obj_read_lock(1)) { + if (!ossl_obj_read_lock()) { ERR_raise(ERR_LIB_OBJ, ERR_R_UNABLE_TO_GET_READ_LOCK); return NULL; } - if (added != NULL) - adp = lh_ADDED_OBJ_retrieve(added, &ad); - ossl_obj_unlock(1); + adp = lh_ADDED_OBJ_retrieve(added, &ad); + ossl_obj_unlock(); if (adp != NULL) return adp->obj; @@ -383,7 +296,7 @@ static int obj_cmp(const ASN1_OBJECT *const *ap, const unsigned int *bp) IMPLEMENT_OBJ_BSEARCH_CMP_FN(const ASN1_OBJECT *, unsigned int, obj); -static int ossl_obj_obj2nid(const ASN1_OBJECT *a, const int lock) +static int ossl_obj_obj2nid(const ASN1_OBJECT *a) { int nid = NID_undef; const unsigned int *op; @@ -399,18 +312,16 @@ static int ossl_obj_obj2nid(const ASN1_OBJECT *a, const int lock) op = OBJ_bsearch_obj(&a, obj_objs, NUM_OBJ); if (op != NULL) return nid_objs[*op].nid; - if (!ossl_obj_read_lock(lock)) { + if (!ossl_obj_read_lock()) { ERR_raise(ERR_LIB_OBJ, ERR_R_UNABLE_TO_GET_READ_LOCK); return NID_undef; } - if (added != NULL) { - ad.type = ADDED_DATA; - ad.obj = (ASN1_OBJECT *)a; /* casting away const is harmless here */ - adp = lh_ADDED_OBJ_retrieve(added, &ad); - if (adp != NULL) - nid = adp->obj->nid; - } - ossl_obj_unlock(lock); + ad.type = ADDED_DATA; + ad.obj = (ASN1_OBJECT *)a; /* casting away const is harmless here */ + adp = lh_ADDED_OBJ_retrieve(added, &ad); + if (adp != NULL) + nid = adp->obj->nid; + ossl_obj_unlock(); return nid; } @@ -645,18 +556,16 @@ int OBJ_ln2nid(const char *s) op = OBJ_bsearch_ln(&oo, ln_objs, NUM_LN); if (op != NULL) return nid_objs[*op].nid; - if (!ossl_obj_read_lock(1)) { + if (!ossl_obj_read_lock()) { ERR_raise(ERR_LIB_OBJ, ERR_R_UNABLE_TO_GET_READ_LOCK); return NID_undef; } - if (added != NULL) { - ad.type = ADDED_LNAME; - ad.obj = &o; - adp = lh_ADDED_OBJ_retrieve(added, &ad); - if (adp != NULL) - nid = adp->obj->nid; - } - ossl_obj_unlock(1); + ad.type = ADDED_LNAME; + ad.obj = &o; + adp = lh_ADDED_OBJ_retrieve(added, &ad); + if (adp != NULL) + nid = adp->obj->nid; + ossl_obj_unlock(); return nid; } @@ -672,18 +581,16 @@ int OBJ_sn2nid(const char *s) op = OBJ_bsearch_sn(&oo, sn_objs, NUM_SN); if (op != NULL) return nid_objs[*op].nid; - if (!ossl_obj_read_lock(1)) { + if (!ossl_obj_read_lock()) { ERR_raise(ERR_LIB_OBJ, ERR_R_UNABLE_TO_GET_READ_LOCK); return NID_undef; } - if (added != NULL) { - ad.type = ADDED_SNAME; - ad.obj = &o; - adp = lh_ADDED_OBJ_retrieve(added, &ad); - if (adp != NULL) - nid = adp->obj->nid; - } - ossl_obj_unlock(1); + ad.type = ADDED_SNAME; + ad.obj = &o; + adp = lh_ADDED_OBJ_retrieve(added, &ad); + if (adp != NULL) + nid = adp->obj->nid; + ossl_obj_unlock(); return nid; } @@ -808,20 +715,14 @@ int OBJ_create(const char *oid, const char *sn, const char *ln) } } - if (!ossl_obj_write_lock(1)) { - ERR_raise(ERR_LIB_OBJ, ERR_R_UNABLE_TO_GET_WRITE_LOCK); - ASN1_OBJECT_free(tmpoid); - return 0; - } - /* If NID is not NID_undef then object already exists */ if (oid != NULL - && ossl_obj_obj2nid(tmpoid, 0) != NID_undef) { + && ossl_obj_obj2nid(tmpoid) != NID_undef) { ERR_raise(ERR_LIB_OBJ, OBJ_R_OID_EXISTS); goto err; } - tmpoid->nid = obj_new_nid_unlocked(1); + tmpoid->nid = OBJ_new_nid(1); if (tmpoid->nid == NID_undef) goto err; @@ -829,13 +730,14 @@ int OBJ_create(const char *oid, const char *sn, const char *ln) tmpoid->sn = (char *)sn; tmpoid->ln = (char *)ln; - ok = ossl_obj_add_object(tmpoid, 0); + if (OBJ_add_object(tmpoid) != NID_undef) + ok = 1; + tmpoid->sn = NULL; tmpoid->ln = NULL; err: - ossl_obj_unlock(1); ASN1_OBJECT_free(tmpoid); return ok; } @@ -856,10 +758,65 @@ const unsigned char *OBJ_get0_data(const ASN1_OBJECT *obj) int OBJ_add_object(const ASN1_OBJECT *obj) { - return ossl_obj_add_object(obj, 1); + ASN1_OBJECT *o = NULL; + ADDED_OBJ *ao[4] = { NULL, NULL, NULL, NULL }, *aop[4]; + int i; + + if ((o = OBJ_dup(obj)) == NULL) + return NID_undef; + if ((ao[ADDED_NID] = OPENSSL_malloc(sizeof(*ao[0]))) == NULL + || (o->length != 0 + && obj->data != NULL + && (ao[ADDED_DATA] = OPENSSL_malloc(sizeof(*ao[0]))) == NULL) + || (o->sn != NULL + && (ao[ADDED_SNAME] = OPENSSL_malloc(sizeof(*ao[0]))) == NULL) + || (o->ln != NULL + && (ao[ADDED_LNAME] = OPENSSL_malloc(sizeof(*ao[0]))) == NULL)) + goto err2; + + if (!ossl_obj_write_lock()) { + ERR_raise(ERR_LIB_OBJ, ERR_R_UNABLE_TO_GET_WRITE_LOCK); + goto err2; + } + + for (i = ADDED_DATA; i <= ADDED_NID; i++) { + if (ao[i] != NULL) { + ao[i]->type = i; + ao[i]->obj = o; + aop[i] = lh_ADDED_OBJ_retrieve(added, ao[i]); + if (aop[i] != NULL) + aop[i]->type = -1; + (void)lh_ADDED_OBJ_insert(added, ao[i]); + if (lh_ADDED_OBJ_error(added)) { + if (aop[i] != NULL) + aop[i]->type = i; + while (i-- > ADDED_DATA) { + lh_ADDED_OBJ_delete(added, ao[i]); + if (aop[i] != NULL) + aop[i]->type = i; + } + ERR_raise(ERR_LIB_OBJ, ERR_R_CRYPTO_LIB); + goto err; + } + } + } + o->flags &= + ~(ASN1_OBJECT_FLAG_DYNAMIC | ASN1_OBJECT_FLAG_DYNAMIC_STRINGS | + ASN1_OBJECT_FLAG_DYNAMIC_DATA); + + ossl_obj_unlock(); + return o->nid; + + err: + ossl_obj_unlock(); + err2: + for (i = ADDED_DATA; i <= ADDED_NID; i++) + OPENSSL_free(ao[i]); + ASN1_OBJECT_free(o); + return NID_undef; } int OBJ_obj2nid(const ASN1_OBJECT *a) { - return ossl_obj_obj2nid(a, 1); + return ossl_obj_obj2nid(a); }