Fix engine cleanup error handling

Error handling in engine_cleanup_add_first/last was
broken and caused memory leaks.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21971)
This commit is contained in:
Bernd Edlinger 2023-09-05 16:59:45 +02:00
parent fc785a554c
commit 00f2efccf5
4 changed files with 35 additions and 23 deletions

View File

@ -135,28 +135,34 @@ static ENGINE_CLEANUP_ITEM *int_cleanup_item(ENGINE_CLEANUP_CB *cb)
return item;
}
void engine_cleanup_add_first(ENGINE_CLEANUP_CB *cb)
int engine_cleanup_add_first(ENGINE_CLEANUP_CB *cb)
{
ENGINE_CLEANUP_ITEM *item;
if (!int_cleanup_check(1))
return;
item = int_cleanup_item(cb);
if (item != NULL)
if (sk_ENGINE_CLEANUP_ITEM_insert(cleanup_stack, item, 0) <= 0)
OPENSSL_free(item);
}
void engine_cleanup_add_last(ENGINE_CLEANUP_CB *cb)
{
ENGINE_CLEANUP_ITEM *item;
if (!int_cleanup_check(1))
return;
return 0;
item = int_cleanup_item(cb);
if (item != NULL) {
if (sk_ENGINE_CLEANUP_ITEM_push(cleanup_stack, item) <= 0)
OPENSSL_free(item);
if (sk_ENGINE_CLEANUP_ITEM_insert(cleanup_stack, item, 0))
return 1;
OPENSSL_free(item);
}
return 0;
}
int engine_cleanup_add_last(ENGINE_CLEANUP_CB *cb)
{
ENGINE_CLEANUP_ITEM *item;
if (!int_cleanup_check(1))
return 0;
item = int_cleanup_item(cb);
if (item != NULL) {
if (sk_ENGINE_CLEANUP_ITEM_push(cleanup_stack, item) > 0)
return 1;
OPENSSL_free(item);
}
return 0;
}
/* The API function that performs all cleanup */

View File

@ -89,12 +89,16 @@ static int engine_list_add(ENGINE *e)
ERR_raise(ERR_LIB_ENGINE, ENGINE_R_INTERNAL_LIST_ERROR);
return 0;
}
engine_list_head = e;
e->prev = NULL;
/*
* The first time the list allocates, we should register the cleanup.
*/
engine_cleanup_add_last(engine_list_cleanup);
if (!engine_cleanup_add_last(engine_list_cleanup)) {
CRYPTO_DOWN_REF(&e->struct_ref, &ref);
ERR_raise(ERR_LIB_ENGINE, ENGINE_R_INTERNAL_LIST_ERROR);
return 0;
}
engine_list_head = e;
e->prev = NULL;
} else {
/* We are adding to the tail of an existing list. */
if ((engine_list_tail == NULL) || (engine_list_tail->next != NULL)) {

View File

@ -46,8 +46,8 @@ typedef struct st_engine_cleanup_item {
ENGINE_CLEANUP_CB *cb;
} ENGINE_CLEANUP_ITEM;
DEFINE_STACK_OF(ENGINE_CLEANUP_ITEM)
void engine_cleanup_add_first(ENGINE_CLEANUP_CB *cb);
void engine_cleanup_add_last(ENGINE_CLEANUP_CB *cb);
int engine_cleanup_add_first(ENGINE_CLEANUP_CB *cb);
int engine_cleanup_add_last(ENGINE_CLEANUP_CB *cb);
/* We need stacks of ENGINEs for use in eng_table.c */
DEFINE_STACK_OF(ENGINE)

View File

@ -93,9 +93,11 @@ int engine_table_register(ENGINE_TABLE **table, ENGINE_CLEANUP_CB *cleanup,
added = 1;
if (!int_table_check(table, 1))
goto end;
if (added)
/* The cleanup callback needs to be added */
engine_cleanup_add_first(cleanup);
/* The cleanup callback needs to be added */
if (added && !engine_cleanup_add_first(cleanup)) {
lh_ENGINE_PILE_free(&(*table)->piles);
*table = NULL;
}
while (num_nids--) {
tmplate.nid = *nids;
fnd = lh_ENGINE_PILE_retrieve(&(*table)->piles, &tmplate);