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)

2 files changed, 59 insertions, 11 deletions

diff --git a/crypto/lhash/lhash.c b/crypto/lhash/lhash.c
index 0a475b71d8..8d7693d749 100644
--- a/crypto/lhash/lhash.c
+++ b/crypto/lhash/lhash.c
@@ -44,6 +44,22 @@ static int expand(OPENSSL_LHASH *lh);
 static void contract(OPENSSL_LHASH *lh);
 static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh, const void *data, unsigned long *rhash);
 
+OPENSSL_LHASH *OPENSSL_LH_set_thunks(OPENSSL_LHASH *lh,
+                                     OPENSSL_LH_HASHFUNCTHUNK hw,
+                                     OPENSSL_LH_COMPFUNCTHUNK cw,
+                                     OPENSSL_LH_DOALL_FUNC_THUNK daw,
+                                     OPENSSL_LH_DOALL_FUNCARG_THUNK daaw)
+{
+
+    if (lh == NULL)
+        return NULL;
+    lh->compw = cw;
+    lh->hashw = hw;
+    lh->daw = daw;
+    lh->daaw = daaw;
+    return lh;
+}
+
 OPENSSL_LHASH *OPENSSL_LH_new(OPENSSL_LH_HASHFUNC h, OPENSSL_LH_COMPFUNC c)
 {
     OPENSSL_LHASH *ret;
@@ -168,8 +184,11 @@ void *OPENSSL_LH_retrieve(OPENSSL_LHASH *lh, const void *data)
 }
 
 static void doall_util_fn(OPENSSL_LHASH *lh, int use_arg,
+                          OPENSSL_LH_DOALL_FUNC_THUNK wfunc,
                           OPENSSL_LH_DOALL_FUNC func,
-                          OPENSSL_LH_DOALL_FUNCARG func_arg, void *arg)
+                          OPENSSL_LH_DOALL_FUNCARG func_arg,
+                          OPENSSL_LH_DOALL_FUNCARG_THUNK wfunc_arg,
+                          void *arg)
 {
     int i;
     OPENSSL_LH_NODE *a, *n;
@@ -186,9 +205,9 @@ static void doall_util_fn(OPENSSL_LHASH *lh, int use_arg,
         while (a != NULL) {
             n = a->next;
             if (use_arg)
-                func_arg(a->data, arg);
+                wfunc_arg(a->data, arg, func_arg);
             else
-                func(a->data);
+                wfunc(a->data, func);
             a = n;
         }
     }
@@ -196,12 +215,29 @@ static void doall_util_fn(OPENSSL_LHASH *lh, int use_arg,
 
 void OPENSSL_LH_doall(OPENSSL_LHASH *lh, OPENSSL_LH_DOALL_FUNC func)
 {
-    doall_util_fn(lh, 0, func, (OPENSSL_LH_DOALL_FUNCARG)0, NULL);
+    if (lh == NULL)
+        return;
+
+    doall_util_fn(lh, 0, lh->daw, func, (OPENSSL_LH_DOALL_FUNCARG)NULL,
+                  (OPENSSL_LH_DOALL_FUNCARG_THUNK)NULL, NULL);
 }
 
-void OPENSSL_LH_doall_arg(OPENSSL_LHASH *lh, OPENSSL_LH_DOALL_FUNCARG func, void *arg)
+void OPENSSL_LH_doall_arg(OPENSSL_LHASH *lh,
+                          OPENSSL_LH_DOALL_FUNCARG func, void *arg)
 {
-    doall_util_fn(lh, 1, (OPENSSL_LH_DOALL_FUNC)0, func, arg);
+    if (lh == NULL)
+        return;
+
+    doall_util_fn(lh, 1, (OPENSSL_LH_DOALL_FUNC_THUNK)NULL,
+                  (OPENSSL_LH_DOALL_FUNC)NULL, func, lh->daaw, arg);
+}
+
+void OPENSSL_LH_doall_arg_thunk(OPENSSL_LHASH *lh,
+                                OPENSSL_LH_DOALL_FUNCARG_THUNK daaw,
+                                OPENSSL_LH_DOALL_FUNCARG fn, void *arg)
+{
+    doall_util_fn(lh, 1, (OPENSSL_LH_DOALL_FUNC_THUNK)NULL,
+                  (OPENSSL_LH_DOALL_FUNC)NULL, fn, daaw, arg);
 }
 
 static int expand(OPENSSL_LHASH *lh)
@@ -286,24 +322,32 @@ static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh,
 {
     OPENSSL_LH_NODE **ret, *n1;
     unsigned long hash, nn;
-    OPENSSL_LH_COMPFUNC cf;
 
-    hash = (*(lh->hash)) (data);
+    if (lh->hashw != NULL)
+        hash = lh->hashw(data, lh->hash);
+    else
+        hash = lh->hash(data);
+
     *rhash = hash;
 
     nn = hash % lh->pmax;
     if (nn < lh->p)
         nn = hash % lh->num_alloc_nodes;
 
-    cf = lh->comp;
     ret = &(lh->b[(int)nn]);
     for (n1 = *ret; n1 != NULL; n1 = n1->next) {
         if (n1->hash != hash) {
             ret = &(n1->next);
             continue;
         }
-        if (cf(n1->data, data) == 0)
-            break;
+
+        if (lh->compw != NULL) {
+            if (lh->compw(n1->data, data, lh->comp) == 0)
+                break;
+        } else {
+            if (lh->comp(n1->data, data) == 0)
+                break;
+        }
         ret = &(n1->next);
     }
     return ret;
diff --git a/crypto/lhash/lhash_local.h b/crypto/lhash/lhash_local.h
index 088ac94d2e..5d9a0cb0ec 100644
--- a/crypto/lhash/lhash_local.h
+++ b/crypto/lhash/lhash_local.h
@@ -20,6 +20,10 @@ struct lhash_st {
     OPENSSL_LH_NODE **b;
     OPENSSL_LH_COMPFUNC comp;
     OPENSSL_LH_HASHFUNC hash;
+    OPENSSL_LH_HASHFUNCTHUNK hashw;
+    OPENSSL_LH_COMPFUNCTHUNK compw;
+    OPENSSL_LH_DOALL_FUNC_THUNK daw;
+    OPENSSL_LH_DOALL_FUNCARG_THUNK daaw;
     unsigned int num_nodes;
     unsigned int num_alloc_nodes;
     unsigned int p;