mirror/openssl
2
0
Fork 0
mirror of https://github.com/openssl/openssl.git synced 2025-02-23 14:42:15 +08:00
Code Issues Packages Projects Releases Wiki Activity

Convert the ENGINE struct_ref field to be an atomic

Browse Source 
We use atomic primitives to up ref and down the struct_ref field rather
than relying on the global lock for this.

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20950)
This commit is contained in:
Matt Caswell 2023-05-11 14:14:31 +01:00 committed by Tomas Mraz
parent 68668243b1
commit e568d64f9f
6 changed files with 75 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
crypto/engine/eng_ctrl.c
View File

@ -127,20 +127,15 @@ static int int_ctrl_helper(ENGINE *e, int cmd, long i, void *p,
int ENGINE_ctrl(ENGINE *e, int cmd, long i, void *p, void (*f) (void))
{
int ctrl_exists, ref_exists;
int ctrl_exists;
if (e == NULL) {
ERR_raise(ERR_LIB_ENGINE, ERR_R_PASSED_NULL_PARAMETER);
return 0;
}
if (!CRYPTO_THREAD_write_lock(global_engine_lock))
return 0;
ref_exists = ((e->struct_ref > 0) ? 1 : 0);
CRYPTO_THREAD_unlock(global_engine_lock);
ctrl_exists = ((e->ctrl == NULL) ? 0 : 1);
if (!ref_exists) {
ERR_raise(ERR_LIB_ENGINE, ENGINE_R_NO_REFERENCE);
return 0;
}
/*
* Intercept any "root-level" commands before trying to hand them on to
* ctrl() handlers.

7
crypto/engine/eng_init.c
View File

@ -28,11 +28,16 @@ int engine_unlocked_init(ENGINE *e)
*/
to_return = e->init(e);
if (to_return) {
int ref;
/*
* OK, we return a functional reference which is also a structural
* reference.
*/
e->struct_ref++;
if (!CRYPTO_UP_REF(&e->struct_ref, &ref, e->refcnt_lock)) {
e->finish(e);
return 0;
}
e->funct_ref++;
ENGINE_REF_PRINT(e, 0, 1);
ENGINE_REF_PRINT(e, 1, 1);

12
crypto/engine/eng_lib.c
View File

@ -37,7 +37,13 @@ ENGINE *ENGINE_new(void)
return NULL;
ret->struct_ref = 1;
ENGINE_REF_PRINT(ret, 0, 1);
ret->refcnt_lock = CRYPTO_THREAD_lock_new();
if (ret->refcnt_lock == NULL) {
OPENSSL_free(ret);
return NULL;
}
if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_ENGINE, ret, &ret->ex_data)) {
CRYPTO_THREAD_lock_free(ret->refcnt_lock);
OPENSSL_free(ret);
return NULL;
}
@ -76,10 +82,7 @@ int engine_free_util(ENGINE *e, int not_locked)
if (e == NULL)
return 1;
if (not_locked)
CRYPTO_DOWN_REF(&e->struct_ref, &i, global_engine_lock);
else
i = --e->struct_ref;
CRYPTO_DOWN_REF(&e->struct_ref, &i, e->refcnt_lock);
ENGINE_REF_PRINT(e, 0, -1);
if (i > 0)
return 1;
@ -95,6 +98,7 @@ int engine_free_util(ENGINE *e, int not_locked)
e->destroy(e);
engine_remove_dynamic_id(e, not_locked);
CRYPTO_free_ex_data(CRYPTO_EX_INDEX_ENGINE, e, &e->ex_data);
CRYPTO_THREAD_lock_free(e->refcnt_lock);
OPENSSL_free(e);
return 1;
}

57
crypto/engine/eng_list.c
View File

@ -58,6 +58,7 @@ static int engine_list_add(ENGINE *e)
{
int conflict = 0;
ENGINE *iterator = NULL;
int ref;
if (e == NULL) {
ERR_raise(ERR_LIB_ENGINE, ERR_R_PASSED_NULL_PARAMETER);
@ -72,9 +73,19 @@ static int engine_list_add(ENGINE *e)
ERR_raise(ERR_LIB_ENGINE, ENGINE_R_CONFLICTING_ENGINE_ID);
return 0;
}
/*
* Having the engine in the list assumes a structural reference.
*/
if (!CRYPTO_UP_REF(&e->struct_ref, &ref, e->refcnt_lock)) {
ERR_raise(ERR_LIB_ENGINE, ENGINE_R_INTERNAL_LIST_ERROR);
return 0;
}
ENGINE_REF_PRINT(e, 0, 1);
if (engine_list_head == NULL) {
/* We are adding to an empty list. */
if (engine_list_tail) {
if (engine_list_tail != NULL) {
CRYPTO_DOWN_REF(&e->struct_ref, &ref, e->refcnt_lock);
ERR_raise(ERR_LIB_ENGINE, ENGINE_R_INTERNAL_LIST_ERROR);
return 0;
}
@ -87,17 +98,14 @@ static int engine_list_add(ENGINE *e)
} else {
/* We are adding to the tail of an existing list. */
if ((engine_list_tail == NULL) || (engine_list_tail->next != NULL)) {
CRYPTO_DOWN_REF(&e->struct_ref, &ref, e->refcnt_lock);
ERR_raise(ERR_LIB_ENGINE, ENGINE_R_INTERNAL_LIST_ERROR);
return 0;
}
engine_list_tail->next = e;
e->prev = engine_list_tail;
}
/*
* Having the engine in the list assumes a structural reference.
*/
e->struct_ref++;
ENGINE_REF_PRINT(e, 0, 1);
/* However it came to be, e is the last item in the list. */
engine_list_tail = e;
e->next = NULL;
@ -228,7 +236,12 @@ ENGINE *ENGINE_get_first(void)
return NULL;
ret = engine_list_head;
if (ret) {
ret->struct_ref++;
int ref;
if (!CRYPTO_UP_REF(&ret->struct_ref, &ref, ret->refcnt_lock)) {
ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
return NULL;
}
ENGINE_REF_PRINT(ret, 0, 1);
}
CRYPTO_THREAD_unlock(global_engine_lock);
@ -249,7 +262,12 @@ ENGINE *ENGINE_get_last(void)
return NULL;
ret = engine_list_tail;
if (ret) {
ret->struct_ref++;
int ref;
if (!CRYPTO_UP_REF(&ret->struct_ref, &ref, ret->refcnt_lock)) {
ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
return NULL;
}
ENGINE_REF_PRINT(ret, 0, 1);
}
CRYPTO_THREAD_unlock(global_engine_lock);
@ -268,8 +286,13 @@ ENGINE *ENGINE_get_next(ENGINE *e)
return NULL;
ret = e->next;
if (ret) {
int ref;
/* Return a valid structural reference to the next ENGINE */
ret->struct_ref++;
if (!CRYPTO_UP_REF(&ret->struct_ref, &ref, ret->refcnt_lock)) {
ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
return NULL;
}
ENGINE_REF_PRINT(ret, 0, 1);
}
CRYPTO_THREAD_unlock(global_engine_lock);
@ -289,8 +312,13 @@ ENGINE *ENGINE_get_prev(ENGINE *e)
return NULL;
ret = e->prev;
if (ret) {
int ref;
/* Return a valid structural reference to the next ENGINE */
ret->struct_ref++;
if (!CRYPTO_UP_REF(&ret->struct_ref, &ref, ret->refcnt_lock)) {
ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
return NULL;
}
ENGINE_REF_PRINT(ret, 0, 1);
}
CRYPTO_THREAD_unlock(global_engine_lock);
@ -405,7 +433,14 @@ ENGINE *ENGINE_by_id(const char *id)
iterator = cp;
}
} else {
iterator->struct_ref++;
int ref;
if (!CRYPTO_UP_REF(&iterator->struct_ref, &ref,
iterator->refcnt_lock)) {
CRYPTO_THREAD_unlock(global_engine_lock);
ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
return NULL;
}
ENGINE_REF_PRINT(iterator, 0, 1);
}
}

4
crypto/engine/eng_local.h
View File

@ -23,7 +23,8 @@ extern CRYPTO_RWLOCK *global_engine_lock;
* This prints the engine's pointer address, "struct" or "funct" to
* indicate the reference type, the before and after reference count, and
* the file:line-number pair. The "ENGINE_REF_PRINT" statements must come
* *after* the change.
* *after* the change. Since this is for tracing only we do not concern
* ourselves with using atomic primitives for reading the struct_ref
*/
# define ENGINE_REF_PRINT(e, isfunct, diff) \
OSSL_TRACE6(ENGINE_REF_COUNT, \
@ -135,6 +136,7 @@ struct engine_st {
int flags;
/* reference count on the structure itself */
CRYPTO_REF_COUNT struct_ref;
CRYPTO_RWLOCK *refcnt_lock;
/*
* reference count on usability of the engine type. NB: This controls the
* loading and initialisation of any functionality required by this

10
crypto/engine/tb_asnmth.c
View File

@ -205,8 +205,14 @@ const EVP_PKEY_ASN1_METHOD *ENGINE_pkey_asn1_find_str(ENGINE **pe,
return NULL;
engine_table_doall(pkey_asn1_meth_table, look_str_cb, &fstr);
/* If found obtain a structural reference to engine */
if (fstr.e) {
fstr.e->struct_ref++;
if (fstr.e != NULL) {
int ref;
if (!CRYPTO_UP_REF(&fstr.e->struct_ref, &ref, fstr.e->refcnt_lock)) {
CRYPTO_THREAD_unlock(global_engine_lock);
ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
return NULL;
}
ENGINE_REF_PRINT(fstr.e, 0, 1);
}
*pe = fstr.e;