summaryrefslogtreecommitdiffstats
path: root/util
diff options
context:
space:
mode:
authorNeil Horman <nhorman@openssl.org>2023-12-02 22:22:32 -0500
committerNeil Horman <nhorman@openssl.org>2024-01-17 10:47:04 -0500
commit5c42ced0ff974a59af98b75e54136f4282718266 (patch)
tree09be6d4eaaf10aacd19973ec46b1650587cf24d2 /util
parent2464d8dc1907b832222ec19d1d0500d9306c47d5 (diff)
Introduce hash thunking functions to do proper casting
ubsan on clang17 has started warning about the following undefined behavior: crypto/lhash/lhash.c:299:12: runtime error: call to function err_string_data_hash through pointer to incorrect function type 'unsigned long (*)(const void *)' [...]/crypto/err/err.c:184: note: err_string_data_hash defined here #0 0x7fa569e3a434 in getrn [...]/crypto/lhash/lhash.c:299:12 #1 0x7fa569e39a46 in OPENSSL_LH_insert [...]/crypto/lhash/lhash.c:119:10 #2 0x7fa569d866ee in err_load_strings [...]/crypto/err/err.c:280:15 [...] The issue occurs because, the generic hash functions (OPENSSL_LH_*) will occasionaly call back to the type specific registered functions for hash generation/comparison/free/etc, using functions of the (example) prototype: [return value] <hash|cmp|free> (void *, [void *], ...) While the functions implementing hash|cmp|free|etc are defined as [return value] <fnname> (TYPE *, [TYPE *], ...) The compiler, not knowing the type signature of the function pointed to by the implementation, performs no type conversion on the function arguments While the C language specification allows for pointers to data of one type to be converted to pointers of another type, it does not allow for pointers to functions with one signature to be called while pointing to functions of another signature. Compilers often allow this behavior, but strictly speaking it results in undefined behavior As such, ubsan warns us about this issue This is an potential fix for the issue, implemented using, in effect, thunking macros. For each hash type, an additional set of wrapper funtions is created (currently for compare and hash, but more will be added for free/doall/etc). The corresponding thunking macros for each type cases the actuall corresponding callback to a function pointer of the proper type, and then calls that with the parameters appropriately cast, avoiding the ubsan warning This approach is adventageous as it maintains a level of type safety, but comes at the cost of having to implement several additional functions per hash table type. Related to #22896 Reviewed-by: Sasa Nedvedicky <sashan@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/23192)
Diffstat (limited to 'util')
-rw-r--r--util/libcrypto.num2
-rw-r--r--util/perl/OpenSSL/stackhash.pm2
2 files changed, 3 insertions, 1 deletions
diff --git a/util/libcrypto.num b/util/libcrypto.num
index f6e5a90ad7..7373b002b7 100644
--- a/util/libcrypto.num
+++ b/util/libcrypto.num
@@ -5544,3 +5544,5 @@ OSSL_CMP_SRV_CTX_init_trans ? 3_3_0 EXIST::FUNCTION:CMP
EVP_DigestSqueeze ? 3_3_0 EXIST::FUNCTION:
ERR_pop ? 3_3_0 EXIST::FUNCTION:
X509_STORE_get1_objects ? 3_3_0 EXIST::FUNCTION:
+OPENSSL_LH_set_thunks ? 3_3_0 EXIST::FUNCTION:
+OPENSSL_LH_doall_arg_thunk ? 3_3_0 EXIST::FUNCTION:
diff --git a/util/perl/OpenSSL/stackhash.pm b/util/perl/OpenSSL/stackhash.pm
index 4d59eab0c9..6c503f29cc 100644
--- a/util/perl/OpenSSL/stackhash.pm
+++ b/util/perl/OpenSSL/stackhash.pm
@@ -85,7 +85,7 @@ sub generate_lhash_macros {
my $macros = <<END_MACROS;
DEFINE_LHASH_OF_INTERNAL(${type});
-#define lh_${type}_new(hfn, cmp) ((LHASH_OF(${type}) *)OPENSSL_LH_new(ossl_check_${type}_lh_hashfunc_type(hfn), ossl_check_${type}_lh_compfunc_type(cmp)))
+#define lh_${type}_new(hfn, cmp) ((LHASH_OF(${type}) *)OPENSSL_LH_set_thunks(OPENSSL_LH_new(ossl_check_${type}_lh_hashfunc_type(hfn), ossl_check_${type}_lh_compfunc_type(cmp)), lh_${type}_hash_thunk, lh_${type}_comp_thunk, lh_${type}_doall_thunk, lh_${type}_doall_arg_thunk))
#define lh_${type}_free(lh) OPENSSL_LH_free(ossl_check_${type}_lh_type(lh))
#define lh_${type}_flush(lh) OPENSSL_LH_flush(ossl_check_${type}_lh_type(lh))
#define lh_${type}_insert(lh, ptr) ((${type} *)OPENSSL_LH_insert(ossl_check_${type}_lh_type(lh), ossl_check_${type}_lh_plain_type(ptr)))