From ecfa839164864ae716c339bb3d6a9e6c596fd973 Mon Sep 17 00:00:00 2001 From: jsing Date: Wed, 2 Oct 2024 14:54:26 +0000 Subject: [PATCH] Remove err_fns and associated machinery. Like all good OpenSSL code, errors was built to be completely extensible. Thankfully, the ERR_{get,set}_implementation() functions were removed in r1.127 of err.c, which means that the extensibility can no longer be used. Take the first of many steps to clean up this code - remove err_fns and associated machinery, calling functions directly. Rename so that we have an 'err_' prefix rather than 'int_' (or nothing). ok joshua@ tb@ --- lib/libcrypto/err/err.c | 207 +++++++++++++--------------------------- 1 file changed, 65 insertions(+), 142 deletions(-) diff --git a/lib/libcrypto/err/err.c b/lib/libcrypto/err/err.c index d8ad4f8bab1..3babdb3211d 100644 --- a/lib/libcrypto/err/err.c +++ b/lib/libcrypto/err/err.c @@ -1,4 +1,4 @@ -/* $OpenBSD: err.c,v 1.63 2024/08/31 10:09:15 tb Exp $ */ +/* $OpenBSD: err.c,v 1.64 2024/10/02 14:54:26 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -238,87 +238,24 @@ static ERR_STRING_DATA ERR_str_reasons[] = { }; #endif - -/* Define the predeclared (but externally opaque) "ERR_FNS" type */ -struct st_ERR_FNS { - /* Works on the "error_hash" string table */ - LHASH_OF(ERR_STRING_DATA) *(*cb_err_get)(int create); - void (*cb_err_del)(void); - const ERR_STRING_DATA *(*cb_err_get_item)(const ERR_STRING_DATA *); - const ERR_STRING_DATA *(*cb_err_set_item)(const ERR_STRING_DATA *); - const ERR_STRING_DATA *(*cb_err_del_item)(const ERR_STRING_DATA *); - /* Works on the "thread_hash" error-state table */ - LHASH_OF(ERR_STATE) *(*cb_thread_get)(int create); - void (*cb_thread_release)(LHASH_OF(ERR_STATE) **hash); - ERR_STATE *(*cb_thread_get_item)(const ERR_STATE *); - ERR_STATE *(*cb_thread_set_item)(ERR_STATE *); - void (*cb_thread_del_item)(const ERR_STATE *); - /* Returns the next available error "library" numbers */ - int (*cb_get_next_lib)(void); -}; - -/* Predeclarations of the "err_defaults" functions */ -static LHASH_OF(ERR_STRING_DATA) *int_err_get(int create); -static void int_err_del(void); -static const ERR_STRING_DATA *int_err_get_item(const ERR_STRING_DATA *); -static const ERR_STRING_DATA *int_err_set_item(const ERR_STRING_DATA *); -static const ERR_STRING_DATA *int_err_del_item(const ERR_STRING_DATA *); -static LHASH_OF(ERR_STATE) *int_thread_get(int create); -static void int_thread_release(LHASH_OF(ERR_STATE) **hash); -static ERR_STATE *int_thread_get_item(const ERR_STATE *); -static ERR_STATE *int_thread_set_item(ERR_STATE *); -static void int_thread_del_item(const ERR_STATE *); -static int int_err_get_next_lib(void); - -/* The static ERR_FNS table using these defaults functions */ -static const ERR_FNS err_defaults = { - int_err_get, - int_err_del, - int_err_get_item, - int_err_set_item, - int_err_del_item, - int_thread_get, - int_thread_release, - int_thread_get_item, - int_thread_set_item, - int_thread_del_item, - int_err_get_next_lib -}; - -/* The replacable table of ERR_FNS functions we use at run-time */ -static const ERR_FNS *err_fns = NULL; - -/* Eg. rather than using "err_get()", use "ERRFN(err_get)()". */ -#define ERRFN(a) err_fns->cb_##a - -/* The internal state used by "err_defaults" - as such, the setting, reading, +/* + * The internal state used by "err_defaults" - as such, the setting, reading, * creating, and deleting of this data should only be permitted via the * "err_defaults" functions. This way, a linked module can completely defer all * ERR state operation (together with requisite locking) to the implementations - * and state in the loading application. */ -static LHASH_OF(ERR_STRING_DATA) *int_error_hash = NULL; -static LHASH_OF(ERR_STATE) *int_thread_hash = NULL; -static int int_thread_hash_references = 0; -static int int_err_library_number = ERR_LIB_USER; + * and state in the loading application. + */ +static LHASH_OF(ERR_STRING_DATA) *err_error_hash = NULL; +static LHASH_OF(ERR_STATE) *err_thread_hash = NULL; +static int err_thread_hash_references = 0; +static int err_library_number = ERR_LIB_USER; static pthread_t err_init_thread; -/* Internal function that checks whether "err_fns" is set and if not, sets it to - * the defaults. */ -static void -err_fns_check(void) -{ - if (err_fns) - return; - - CRYPTO_w_lock(CRYPTO_LOCK_ERR); - if (!err_fns) - err_fns = &err_defaults; - CRYPTO_w_unlock(CRYPTO_LOCK_ERR); -} - -/* These are the callbacks provided to "lh_new()" when creating the LHASH tables - * internal to the "err_defaults" implementation. */ +/* + * These are the callbacks provided to "lh_new()" when creating the LHASH tables + * internal to the "err_defaults" implementation. + */ static unsigned long get_error_values(int inc, int top, const char **file, int *line, const char **data, int *flags); @@ -344,39 +281,38 @@ err_string_data_cmp(const ERR_STRING_DATA *a, const ERR_STRING_DATA *b) static IMPLEMENT_LHASH_COMP_FN(err_string_data, ERR_STRING_DATA) static LHASH_OF(ERR_STRING_DATA) * -int_err_get(int create) +err_get(int create) { LHASH_OF(ERR_STRING_DATA) *ret = NULL; CRYPTO_w_lock(CRYPTO_LOCK_ERR); - if (!int_error_hash && create) - int_error_hash = lh_ERR_STRING_DATA_new(); - if (int_error_hash) - ret = int_error_hash; + if (!err_error_hash && create) + err_error_hash = lh_ERR_STRING_DATA_new(); + if (err_error_hash) + ret = err_error_hash; CRYPTO_w_unlock(CRYPTO_LOCK_ERR); return ret; } static void -int_err_del(void) +err_del(void) { CRYPTO_w_lock(CRYPTO_LOCK_ERR); - if (int_error_hash) { - lh_ERR_STRING_DATA_free(int_error_hash); - int_error_hash = NULL; + if (err_error_hash) { + lh_ERR_STRING_DATA_free(err_error_hash); + err_error_hash = NULL; } CRYPTO_w_unlock(CRYPTO_LOCK_ERR); } static const ERR_STRING_DATA * -int_err_get_item(const ERR_STRING_DATA *d) +err_get_item(const ERR_STRING_DATA *d) { ERR_STRING_DATA *p; LHASH_OF(ERR_STRING_DATA) *hash; - err_fns_check(); - hash = ERRFN(err_get)(0); + hash = err_get(0); if (!hash) return NULL; @@ -388,13 +324,12 @@ int_err_get_item(const ERR_STRING_DATA *d) } static const ERR_STRING_DATA * -int_err_set_item(const ERR_STRING_DATA *d) +err_set_item(const ERR_STRING_DATA *d) { const ERR_STRING_DATA *p; LHASH_OF(ERR_STRING_DATA) *hash; - err_fns_check(); - hash = ERRFN(err_get)(1); + hash = err_get(1); if (!hash) return NULL; @@ -406,13 +341,12 @@ int_err_set_item(const ERR_STRING_DATA *d) } static const ERR_STRING_DATA * -int_err_del_item(const ERR_STRING_DATA *d) +err_del_item(const ERR_STRING_DATA *d) { ERR_STRING_DATA *p; LHASH_OF(ERR_STRING_DATA) *hash; - err_fns_check(); - hash = ERRFN(err_get)(0); + hash = err_get(0); if (!hash) return NULL; @@ -438,30 +372,30 @@ err_state_cmp(const ERR_STATE *a, const ERR_STATE *b) static IMPLEMENT_LHASH_COMP_FN(err_state, ERR_STATE) static LHASH_OF(ERR_STATE) * -int_thread_get(int create) +err_thread_get(int create) { LHASH_OF(ERR_STATE) *ret = NULL; CRYPTO_w_lock(CRYPTO_LOCK_ERR); - if (!int_thread_hash && create) - int_thread_hash = lh_ERR_STATE_new(); - if (int_thread_hash) { - int_thread_hash_references++; - ret = int_thread_hash; + if (!err_thread_hash && create) + err_thread_hash = lh_ERR_STATE_new(); + if (err_thread_hash) { + err_thread_hash_references++; + ret = err_thread_hash; } CRYPTO_w_unlock(CRYPTO_LOCK_ERR); return ret; } static void -int_thread_release(LHASH_OF(ERR_STATE) **hash) +err_thread_release(LHASH_OF(ERR_STATE) **hash) { int i; if (hash == NULL || *hash == NULL) return; - i = CRYPTO_add(&int_thread_hash_references, -1, CRYPTO_LOCK_ERR); + i = CRYPTO_add(&err_thread_hash_references, -1, CRYPTO_LOCK_ERR); if (i > 0) return; @@ -469,13 +403,12 @@ int_thread_release(LHASH_OF(ERR_STATE) **hash) } static ERR_STATE * -int_thread_get_item(const ERR_STATE *d) +err_thread_get_item(const ERR_STATE *d) { ERR_STATE *p; LHASH_OF(ERR_STATE) *hash; - err_fns_check(); - hash = ERRFN(thread_get)(0); + hash = err_thread_get(0); if (!hash) return NULL; @@ -483,18 +416,17 @@ int_thread_get_item(const ERR_STATE *d) p = lh_ERR_STATE_retrieve(hash, d); CRYPTO_r_unlock(CRYPTO_LOCK_ERR); - ERRFN(thread_release)(&hash); + err_thread_release(&hash); return p; } static ERR_STATE * -int_thread_set_item(ERR_STATE *d) +err_thread_set_item(ERR_STATE *d) { ERR_STATE *p; LHASH_OF(ERR_STATE) *hash; - err_fns_check(); - hash = ERRFN(thread_get)(1); + hash = err_thread_get(1); if (!hash) return NULL; @@ -502,43 +434,42 @@ int_thread_set_item(ERR_STATE *d) p = lh_ERR_STATE_insert(hash, d); CRYPTO_w_unlock(CRYPTO_LOCK_ERR); - ERRFN(thread_release)(&hash); + err_thread_release(&hash); return p; } static void -int_thread_del_item(const ERR_STATE *d) +err_thread_del_item(const ERR_STATE *d) { ERR_STATE *p; LHASH_OF(ERR_STATE) *hash; - err_fns_check(); - hash = ERRFN(thread_get)(0); + hash = err_thread_get(0); if (!hash) return; CRYPTO_w_lock(CRYPTO_LOCK_ERR); p = lh_ERR_STATE_delete(hash, d); /* make sure we don't leak memory */ - if (int_thread_hash_references == 1 && - int_thread_hash && lh_ERR_STATE_num_items(int_thread_hash) == 0) { - lh_ERR_STATE_free(int_thread_hash); - int_thread_hash = NULL; + if (err_thread_hash_references == 1 && + err_thread_hash && lh_ERR_STATE_num_items(err_thread_hash) == 0) { + lh_ERR_STATE_free(err_thread_hash); + err_thread_hash = NULL; } CRYPTO_w_unlock(CRYPTO_LOCK_ERR); - ERRFN(thread_release)(&hash); + err_thread_release(&hash); if (p) ERR_STATE_free(p); } static int -int_err_get_next_lib(void) +err_get_next_lib(void) { int ret; CRYPTO_w_lock(CRYPTO_LOCK_ERR); - ret = int_err_library_number++; + ret = err_library_number++; CRYPTO_w_unlock(CRYPTO_LOCK_ERR); return ret; @@ -647,7 +578,6 @@ void ERR_load_ERR_strings_internal(void) { err_init_thread = pthread_self(); - err_fns_check(); #ifndef OPENSSL_NO_ERR err_load_strings(0, ERR_str_libraries); err_load_strings(0, ERR_str_reasons); @@ -679,7 +609,7 @@ err_load_strings(int lib, ERR_STRING_DATA *str) while (str->error) { if (lib) str->error |= ERR_PACK(lib, 0, 0); - ERRFN(err_set_item)(str); + err_set_item(str); str++; } } @@ -697,7 +627,7 @@ ERR_load_const_strings(const ERR_STRING_DATA *str) { ERR_load_ERR_strings(); while (str->error) { - ERRFN(err_set_item)(str); + err_set_item(str); str++; } } @@ -711,7 +641,7 @@ ERR_unload_strings(int lib, ERR_STRING_DATA *str) while (str->error) { if (lib) str->error |= ERR_PACK(lib, 0, 0); - ERRFN(err_del_item)(str); + err_del_item(str); str++; } } @@ -723,8 +653,7 @@ ERR_free_strings(void) /* Prayer and clean living lets you ignore errors, OpenSSL style */ (void) OPENSSL_init_crypto(0, NULL); - err_fns_check(); - ERRFN(err_del)(); + err_del(); } LCRYPTO_ALIAS(ERR_free_strings); @@ -981,10 +910,9 @@ ERR_lib_error_string(unsigned long e) if (!OPENSSL_init_crypto(0, NULL)) return NULL; - err_fns_check(); l = ERR_GET_LIB(e); d.error = ERR_PACK(l, 0, 0); - p = ERRFN(err_get_item)(&d); + p = err_get_item(&d); return ((p == NULL) ? NULL : p->string); } LCRYPTO_ALIAS(ERR_lib_error_string); @@ -996,11 +924,10 @@ ERR_func_error_string(unsigned long e) ERR_STRING_DATA d; unsigned long l, f; - err_fns_check(); l = ERR_GET_LIB(e); f = ERR_GET_FUNC(e); d.error = ERR_PACK(l, f, 0); - p = ERRFN(err_get_item)(&d); + p = err_get_item(&d); return ((p == NULL) ? NULL : p->string); } LCRYPTO_ALIAS(ERR_func_error_string); @@ -1012,14 +939,13 @@ ERR_reason_error_string(unsigned long e) ERR_STRING_DATA d; unsigned long l, r; - err_fns_check(); l = ERR_GET_LIB(e); r = ERR_GET_REASON(e); d.error = ERR_PACK(l, 0, r); - p = ERRFN(err_get_item)(&d); + p = err_get_item(&d); if (!p) { d.error = ERR_PACK(0, 0, r); - p = ERRFN(err_get_item)(&d); + p = err_get_item(&d); } return ((p == NULL) ? NULL : p->string); } @@ -1034,10 +960,9 @@ ERR_remove_thread_state(const CRYPTO_THREADID *id) CRYPTO_THREADID_cpy(&tmp.tid, id); else CRYPTO_THREADID_current(&tmp.tid); - err_fns_check(); - /* thread_del_item automatically destroys the LHASH if the number of + /* err_thread_del_item automatically destroys the LHASH if the number of * items reaches zero. */ - ERRFN(thread_del_item)(&tmp); + err_thread_del_item(&tmp); } LCRYPTO_ALIAS(ERR_remove_thread_state); @@ -1056,10 +981,9 @@ ERR_get_state(void) int i; CRYPTO_THREADID tid; - err_fns_check(); CRYPTO_THREADID_current(&tid); CRYPTO_THREADID_cpy(&tmp.tid, &tid); - ret = ERRFN(thread_get_item)(&tmp); + ret = err_thread_get_item(&tmp); /* ret == the error state, if NULL, make a new one */ if (ret == NULL) { @@ -1073,9 +997,9 @@ ERR_get_state(void) ret->err_data[i] = NULL; ret->err_data_flags[i] = 0; } - tmpp = ERRFN(thread_set_item)(ret); + tmpp = err_thread_set_item(ret); /* To check if insertion failed, do a get. */ - if (ERRFN(thread_get_item)(ret) != ret) { + if (err_thread_get_item(ret) != ret) { ERR_STATE_free(ret); /* could not insert it */ return (&fallback); } @@ -1090,8 +1014,7 @@ ERR_get_state(void) int ERR_get_next_error_library(void) { - err_fns_check(); - return ERRFN(get_next_lib)(); + return err_get_next_lib(); } LCRYPTO_ALIAS(ERR_get_next_error_library); -- 2.20.1