Skip to content

Commit

Permalink
engine: update to structure based atomics
Browse files Browse the repository at this point in the history 
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #21260)
  • Loading branch information
@paulidale
paulidale committed Jul 1, 2023
1 parent 99fd5b2 commit e362070
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 24 deletions.
2 changes: 1 addition & 1 deletion crypto/engine/eng_init.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ int engine_unlocked_init(ENGINE *e)
* OK, we return a functional reference which is also a structural
* reference.
*/
if (!CRYPTO_UP_REF(&e->struct_ref, &ref, e->refcnt_lock)) {
if (!CRYPTO_UP_REF(&e->struct_ref, &ref)) {
e->finish(e);
return 0;
}
Expand Down
12 changes: 5 additions & 7 deletions crypto/engine/eng_lib.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,13 @@ ENGINE *ENGINE_new(void)
}
if ((ret = OPENSSL_zalloc(sizeof(*ret))) == NULL)
return NULL;
ret->struct_ref = 1;
ENGINE_REF_PRINT(ret, 0, 1);
ret->refcnt_lock = CRYPTO_THREAD_lock_new();
if (ret->refcnt_lock == NULL) {
if (!CRYPTO_NEW_REF(&ret->struct_ref, 1)) {
OPENSSL_free(ret);
return NULL;
}
ENGINE_REF_PRINT(ret, 0, 1);
if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_ENGINE, ret, &ret->ex_data)) {
CRYPTO_THREAD_lock_free(ret->refcnt_lock);
CRYPTO_FREE_REF(&ret->struct_ref);
OPENSSL_free(ret);
return NULL;
}
Expand Down Expand Up @@ -82,7 +80,7 @@ int engine_free_util(ENGINE *e, int not_locked)

if (e == NULL)
return 1;
CRYPTO_DOWN_REF(&e->struct_ref, &i, e->refcnt_lock);
CRYPTO_DOWN_REF(&e->struct_ref, &i);
ENGINE_REF_PRINT(e, 0, -1);
if (i > 0)
return 1;
Expand All @@ -98,7 +96,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);
CRYPTO_FREE_REF(&e->struct_ref);
OPENSSL_free(e);
return 1;
}
Expand Down
19 changes: 9 additions & 10 deletions crypto/engine/eng_list.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,15 @@ static int engine_list_add(ENGINE *e)
/*
* Having the engine in the list assumes a structural reference.
*/
if (!CRYPTO_UP_REF(&e->struct_ref, &ref, e->refcnt_lock)) {
if (!CRYPTO_UP_REF(&e->struct_ref, &ref)) {
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 != NULL) {
CRYPTO_DOWN_REF(&e->struct_ref, &ref, e->refcnt_lock);
CRYPTO_DOWN_REF(&e->struct_ref, &ref);
ERR_raise(ERR_LIB_ENGINE, ENGINE_R_INTERNAL_LIST_ERROR);
return 0;
}
Expand All @@ -98,7 +98,7 @@ 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);
CRYPTO_DOWN_REF(&e->struct_ref, &ref);
ERR_raise(ERR_LIB_ENGINE, ENGINE_R_INTERNAL_LIST_ERROR);
return 0;
}
Expand Down Expand Up @@ -238,7 +238,7 @@ ENGINE *ENGINE_get_first(void)
if (ret) {
int ref;

if (!CRYPTO_UP_REF(&ret->struct_ref, &ref, ret->refcnt_lock)) {
if (!CRYPTO_UP_REF(&ret->struct_ref, &ref)) {
ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
return NULL;
}
Expand All @@ -264,7 +264,7 @@ ENGINE *ENGINE_get_last(void)
if (ret) {
int ref;

if (!CRYPTO_UP_REF(&ret->struct_ref, &ref, ret->refcnt_lock)) {
if (!CRYPTO_UP_REF(&ret->struct_ref, &ref)) {
ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
return NULL;
}
Expand All @@ -289,7 +289,7 @@ ENGINE *ENGINE_get_next(ENGINE *e)
int ref;

/* Return a valid structural reference to the next ENGINE */
if (!CRYPTO_UP_REF(&ret->struct_ref, &ref, ret->refcnt_lock)) {
if (!CRYPTO_UP_REF(&ret->struct_ref, &ref)) {
ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
return NULL;
}
Expand All @@ -315,7 +315,7 @@ ENGINE *ENGINE_get_prev(ENGINE *e)
int ref;

/* Return a valid structural reference to the next ENGINE */
if (!CRYPTO_UP_REF(&ret->struct_ref, &ref, ret->refcnt_lock)) {
if (!CRYPTO_UP_REF(&ret->struct_ref, &ref)) {
ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
return NULL;
}
Expand Down Expand Up @@ -435,8 +435,7 @@ ENGINE *ENGINE_by_id(const char *id)
} else {
int ref;

if (!CRYPTO_UP_REF(&iterator->struct_ref, &ref,
iterator->refcnt_lock)) {
if (!CRYPTO_UP_REF(&iterator->struct_ref, &ref)) {
CRYPTO_THREAD_unlock(global_engine_lock);
ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
return NULL;
Expand Down Expand Up @@ -477,6 +476,6 @@ int ENGINE_up_ref(ENGINE *e)
ERR_raise(ERR_LIB_ENGINE, ERR_R_PASSED_NULL_PARAMETER);
return 0;
}
CRYPTO_UP_REF(&e->struct_ref, &i, global_engine_lock);
CRYPTO_UP_REF(&e->struct_ref, &i);
return 1;
}
16 changes: 11 additions & 5 deletions crypto/engine/eng_local.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,16 @@ 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. Since this is for tracing only we do not concern
* ourselves with using atomic primitives for reading the struct_ref
* *after* the change.
*/
# define ENGINE_REF_PRINT(e, isfunct, diff) \
OSSL_TRACE6(ENGINE_REF_COUNT, \
"engine: %p %s from %d to %d (%s:%d)\n", \
(void *)(e), (isfunct ? "funct" : "struct"), \
((isfunct) \
? ((e)->funct_ref - (diff)) \
: ((e)->struct_ref - (diff))), \
((isfunct) ? (e)->funct_ref : (e)->struct_ref), \
: (eng_struct_ref(e) - (diff))), \
((isfunct) ? (e)->funct_ref : eng_struct_ref(e)), \
(OPENSSL_FILE), (OPENSSL_LINE))

/*
Expand Down Expand Up @@ -136,7 +135,6 @@ 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
Expand All @@ -160,4 +158,12 @@ typedef struct st_engine_pile ENGINE_PILE;

DEFINE_LHASH_OF_EX(ENGINE_PILE);

static ossl_unused ossl_inline int eng_struct_ref(ENGINE *e)
{
int res;

CRYPTO_GET_REF(&e->struct_ref, &res);
return res;
}

#endif /* OSSL_CRYPTO_ENGINE_ENG_LOCAL_H */
2 changes: 1 addition & 1 deletion crypto/engine/tb_asnmth.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ const EVP_PKEY_ASN1_METHOD *ENGINE_pkey_asn1_find_str(ENGINE **pe,
if (fstr.e != NULL) {
int ref;

if (!CRYPTO_UP_REF(&fstr.e->struct_ref, &ref, fstr.e->refcnt_lock)) {
if (!CRYPTO_UP_REF(&fstr.e->struct_ref, &ref)) {
CRYPTO_THREAD_unlock(global_engine_lock);
ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
return NULL;
Expand Down

0 comments on commit e362070

Please sign in to comment.