]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
add a compare thunk function to the STACK of macros
authorNeil Horman <nhorman@openssl.org>
Wed, 14 Jan 2026 15:10:21 +0000 (10:10 -0500)
committerNeil Horman <nhorman@openssl.org>
Sat, 7 Feb 2026 18:11:08 +0000 (13:11 -0500)
Now that ossl_bsearch is capable of using a thunking function, lets
create a thunking function to use for the STACK_OF macros.

The problem we're addressing is one that gives rise to ubsan issues.
clang-16 forward have a ubsan test that confirms that the target symbol
that we call through a pointer matches the type of the pointer itself.
for instance

int foo(void *a, void *b)
{
   ...
}

int (*fooptr)(char *ac, int *bc) = foo;

fooptr(&charval, &intval);

is strictly speaking in C undefined behavior (even though in normal
operation this works as expected).  Newer compilers are strict about
this however, as several security frameworks operate with an expectation
that this constraint is met.
See https://github.com/openssl/openssl/issues/22896#issuecomment-1837266357
for details.

So we need to create a thunking function.  The sole purpose of this
thunking function is to accept the "real" comparison function for the
STACK_OF macros, along with the two items to compare of the type that
they are passed as from the calling function, and do the convervsion of
both the comparison function and the data pointers to the types that the
real comparison function expects

So we:
1) Modify the DEFINE_STACK_OF macros to create this thunking function
2) Add an OPENSSL_sk_set_cmp_thunks api to set the comparison function
3) modify the requisite places in the stack code to use the thunking
   function when available

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Nikola Pajkovsky <nikolap@openssl.org>
MergeDate: Sat Feb  7 18:11:14 2026
(Merged from https://github.com/openssl/openssl/pull/29640)

crypto/stack/stack.c
include/openssl/safestack.h.in
include/openssl/stack.h
util/libcrypto.num
util/perl/OpenSSL/stackhash.pm

index 17962393356c7e04dffa1ad0315939f4f3f54c17..b4786619f1fb32720e9788d8d3c85e4864c6b380 100644 (file)
@@ -31,6 +31,7 @@ struct stack_st {
     int sorted;
     int num_alloc;
     OPENSSL_sk_compfunc comp;
+    int (*cmp_thunk)(OPENSSL_sk_compfunc, const void *, const void *);
     OPENSSL_sk_freefunc_thunk free_thunk;
 };
 
@@ -243,8 +244,18 @@ OPENSSL_STACK *OPENSSL_sk_set_thunks(OPENSSL_STACK *st, OPENSSL_sk_freefunc_thun
     return st;
 }
 
+OPENSSL_STACK *OPENSSL_sk_set_cmp_thunks(OPENSSL_STACK *st, int (*c_thunk)(int (*)(const void *, const void *), const void *, const void *))
+{
+    if (st != NULL)
+        st->cmp_thunk = c_thunk;
+
+    return st;
+}
+
 int OPENSSL_sk_insert(OPENSSL_STACK *st, const void *data, int loc)
 {
+    int cmp_ret;
+
     if (st == NULL) {
         ERR_raise(ERR_LIB_CRYPTO, ERR_R_PASSED_NULL_PARAMETER);
         return 0;
@@ -268,11 +279,16 @@ int OPENSSL_sk_insert(OPENSSL_STACK *st, const void *data, int loc)
     st->num++;
     if (st->sorted && st->num > 1) {
         if (st->comp != NULL) {
-            if (loc > 0 && (st->comp(&st->data[loc - 1], &st->data[loc]) > 0))
-                st->sorted = 0;
-            if (loc < st->num - 1
-                && (st->comp(&st->data[loc + 1], &st->data[loc]) < 0))
-                st->sorted = 0;
+            if (loc > 0) {
+                cmp_ret = (st->cmp_thunk == NULL) ? st->comp(&st->data[loc - 1], &st->data[loc]) : st->cmp_thunk(st->comp, &st->data[loc - 1], &st->data[loc]);
+                if (cmp_ret > 0)
+                    st->sorted = 0;
+            }
+            if (loc < st->num - 1) {
+                cmp_ret = (st->cmp_thunk == NULL) ? st->comp(&st->data[loc + 1], &st->data[loc]) : st->cmp_thunk(st->comp, &st->data[loc + 1], &st->data[loc]);
+                if (cmp_ret < 0)
+                    st->sorted = 0;
+            }
         } else {
             st->sorted = 0;
         }
@@ -319,6 +335,7 @@ static int internal_find(const OPENSSL_STACK *st, const void *data,
 {
     const void *r;
     int i, count = 0;
+    int cmp_ret;
     int *pnum = pnum_matched;
 
     if (st == NULL || st->num == 0)
@@ -343,8 +360,9 @@ static int internal_find(const OPENSSL_STACK *st, const void *data,
     if (!st->sorted) {
         int res = -1;
 
-        for (i = 0; i < st->num; i++)
-            if (st->comp(&data, st->data + i) == 0) {
+        for (i = 0; i < st->num; i++) {
+            cmp_ret = (st->cmp_thunk == NULL) ? st->comp(&data, st->data + i) : st->cmp_thunk(st->comp, &data, st->data + i);
+            if (cmp_ret == 0) {
                 if (res == -1)
                     res = i;
                 ++*pnum;
@@ -352,6 +370,7 @@ static int internal_find(const OPENSSL_STACK *st, const void *data,
                 if (pnum_matched == NULL)
                     return i;
             }
+        }
         if (res == -1)
             *pnum = 0;
         return res;
@@ -359,7 +378,7 @@ static int internal_find(const OPENSSL_STACK *st, const void *data,
 
     if (pnum_matched != NULL)
         ret_val_options |= OSSL_BSEARCH_FIRST_VALUE_ON_MATCH;
-    r = ossl_bsearch(&data, st->data, st->num, sizeof(void *), st->comp, NULL,
+    r = ossl_bsearch(&data, st->data, st->num, sizeof(void *), st->comp, st->cmp_thunk,
         ret_val_options);
 
     if (pnum_matched != NULL) {
@@ -368,7 +387,8 @@ static int internal_find(const OPENSSL_STACK *st, const void *data,
             const void **p = (const void **)r;
 
             while (p < st->data + st->num) {
-                if (st->comp(&data, p) != 0)
+                cmp_ret = st->cmp_thunk == NULL ? st->comp(&data, p) : st->cmp_thunk(st->comp, &data, p);
+                if (cmp_ret != 0)
                     break;
                 ++*pnum;
                 ++p;
index 0e72de009eb54ba6d3b7256f06422611eff541b6..42c3ec95ff5af865fdd48649f6876d0cb4043986 100644 (file)
@@ -36,39 +36,47 @@ extern "C" {
 #define STACK_OF(type) struct stack_st_##type
 
 /* Helper macro for internal use */
-#define SKM_DEFINE_STACK_OF_INTERNAL(t1, t2, t3)                                                                \
-    STACK_OF(t1);                                                                                               \
-    typedef int (*sk_##t1##_compfunc)(const t3 *const *a, const t3 *const *b);                                  \
-    typedef void (*sk_##t1##_freefunc)(t3 * a);                                                                 \
-    typedef t3 *(*sk_##t1##_copyfunc)(const t3 *a);                                                             \
-    static ossl_inline void sk_##t1##_freefunc_thunk(OPENSSL_sk_freefunc freefunc_arg, void *ptr)               \
-    {                                                                                                           \
-        sk_##t1##_freefunc freefunc = (sk_##t1##_freefunc)freefunc_arg;                                         \
-        freefunc((t3 *)ptr);                                                                                    \
-    }                                                                                                           \
-    static ossl_unused ossl_inline t2 *ossl_check_##t1##_type(t2 *ptr)                                          \
-    {                                                                                                           \
-        return ptr;                                                                                             \
-    }                                                                                                           \
-    static ossl_unused ossl_inline const OPENSSL_STACK *ossl_check_const_##t1##_sk_type(const STACK_OF(t1) *sk) \
-    {                                                                                                           \
-        return (const OPENSSL_STACK *)sk;                                                                       \
-    }                                                                                                           \
-    static ossl_unused ossl_inline OPENSSL_STACK *ossl_check_##t1##_sk_type(STACK_OF(t1) *sk)                   \
-    {                                                                                                           \
-        return (OPENSSL_STACK *)sk;                                                                             \
-    }                                                                                                           \
-    static ossl_unused ossl_inline OPENSSL_sk_compfunc ossl_check_##t1##_compfunc_type(sk_##t1##_compfunc cmp)  \
-    {                                                                                                           \
-        return (OPENSSL_sk_compfunc)cmp;                                                                        \
-    }                                                                                                           \
-    static ossl_unused ossl_inline OPENSSL_sk_copyfunc ossl_check_##t1##_copyfunc_type(sk_##t1##_copyfunc cpy)  \
-    {                                                                                                           \
-        return (OPENSSL_sk_copyfunc)cpy;                                                                        \
-    }                                                                                                           \
-    static ossl_unused ossl_inline OPENSSL_sk_freefunc ossl_check_##t1##_freefunc_type(sk_##t1##_freefunc fr)   \
-    {                                                                                                           \
-        return (OPENSSL_sk_freefunc)fr;                                                                         \
+#define SKM_DEFINE_STACK_OF_INTERNAL(t1, t2, t3)                                                                         \
+    STACK_OF(t1);                                                                                                        \
+    typedef int (*sk_##t1##_compfunc)(const t3 *const *a, const t3 *const *b);                                           \
+    typedef void (*sk_##t1##_freefunc)(t3 * a);                                                                          \
+    typedef t3 *(*sk_##t1##_copyfunc)(const t3 *a);                                                                      \
+    static ossl_inline void sk_##t1##_freefunc_thunk(OPENSSL_sk_freefunc freefunc_arg, void *ptr)                        \
+    {                                                                                                                    \
+        sk_##t1##_freefunc freefunc = (sk_##t1##_freefunc)freefunc_arg;                                                  \
+        freefunc((t3 *)ptr);                                                                                             \
+    }                                                                                                                    \
+    static ossl_inline int sk_##t1##_cmpfunc_thunk(int (*cmp)(const void *, const void *), const void *a, const void *b) \
+    {                                                                                                                    \
+        int (*realcmp)(const t3 *const *a, const t3 *const *b) = (int (*)(const t3 *const *a, const t3 *const *b))(cmp); \
+        const t3 *const *at = (const t3 *const *)a;                                                                      \
+        const t3 *const *bt = (const t3 *const *)b;                                                                      \
+                                                                                                                         \
+        return realcmp(at, bt);                                                                                          \
+    }                                                                                                                    \
+    static ossl_unused ossl_inline t2 *ossl_check_##t1##_type(t2 *ptr)                                                   \
+    {                                                                                                                    \
+        return ptr;                                                                                                      \
+    }                                                                                                                    \
+    static ossl_unused ossl_inline const OPENSSL_STACK *ossl_check_const_##t1##_sk_type(const STACK_OF(t1) *sk)          \
+    {                                                                                                                    \
+        return (const OPENSSL_STACK *)sk;                                                                                \
+    }                                                                                                                    \
+    static ossl_unused ossl_inline OPENSSL_STACK *ossl_check_##t1##_sk_type(STACK_OF(t1) *sk)                            \
+    {                                                                                                                    \
+        return (OPENSSL_STACK *)sk;                                                                                      \
+    }                                                                                                                    \
+    static ossl_unused ossl_inline OPENSSL_sk_compfunc ossl_check_##t1##_compfunc_type(sk_##t1##_compfunc cmp)           \
+    {                                                                                                                    \
+        return (OPENSSL_sk_compfunc)cmp;                                                                                 \
+    }                                                                                                                    \
+    static ossl_unused ossl_inline OPENSSL_sk_copyfunc ossl_check_##t1##_copyfunc_type(sk_##t1##_copyfunc cpy)           \
+    {                                                                                                                    \
+        return (OPENSSL_sk_copyfunc)cpy;                                                                                 \
+    }                                                                                                                    \
+    static ossl_unused ossl_inline OPENSSL_sk_freefunc ossl_check_##t1##_freefunc_type(sk_##t1##_freefunc fr)            \
+    {                                                                                                                    \
+        return (OPENSSL_sk_freefunc)fr;                                                                                  \
     }
 
 #define SKM_DEFINE_STACK_OF(t1, t2, t3)                                                                                    \
@@ -81,6 +89,14 @@ extern "C" {
         sk_##t1##_freefunc freefunc = (sk_##t1##_freefunc)freefunc_arg;                                                    \
         freefunc((t3 *)ptr);                                                                                               \
     }                                                                                                                      \
+    static ossl_inline int sk_##t1##_cmpfunc_thunk(int (*cmp)(const void *, const void *), const void *a, const void *b)   \
+    {                                                                                                                      \
+        int (*realcmp)(const t3 *const *a, const t3 *const *b) = (int (*)(const t3 *const *a, const t3 *const *b))(cmp);   \
+        const t3 *const *at = (const t3 *const *)a;                                                                        \
+        const t3 *const *bt = (const t3 *const *)b;                                                                        \
+                                                                                                                           \
+        return realcmp(at, bt);                                                                                            \
+    }                                                                                                                      \
     static ossl_unused ossl_inline int sk_##t1##_num(const STACK_OF(t1) *sk)                                               \
     {                                                                                                                      \
         return OPENSSL_sk_num((const OPENSSL_STACK *)sk);                                                                  \
@@ -95,6 +111,7 @@ extern "C" {
         OPENSSL_sk_freefunc_thunk f_thunk;                                                                                 \
                                                                                                                            \
         f_thunk = (OPENSSL_sk_freefunc_thunk)sk_##t1##_freefunc_thunk;                                                     \
+        OPENSSL_sk_set_cmp_thunks(ret, sk_##t1##_cmpfunc_thunk);                                                           \
         return (STACK_OF(t1) *)OPENSSL_sk_set_thunks(ret, f_thunk);                                                        \
     }                                                                                                                      \
     static ossl_unused ossl_inline STACK_OF(t1) *sk_##t1##_new_null(void)                                                  \
@@ -112,6 +129,7 @@ extern "C" {
         OPENSSL_sk_freefunc_thunk f_thunk;                                                                                 \
                                                                                                                            \
         f_thunk = (OPENSSL_sk_freefunc_thunk)sk_##t1##_freefunc_thunk;                                                     \
+        OPENSSL_sk_set_cmp_thunks(ret, sk_##t1##_cmpfunc_thunk);                                                           \
         return (STACK_OF(t1) *)OPENSSL_sk_set_thunks(ret, f_thunk);                                                        \
     }                                                                                                                      \
     static ossl_unused ossl_inline int sk_##t1##_reserve(STACK_OF(t1) *sk, int n)                                          \
index d8e818a258f735dd3bf94af817a4ff3c9f4bb017..297434ec61303d02e552119c7a91790986d57fad 100644 (file)
@@ -36,6 +36,7 @@ OPENSSL_STACK *OPENSSL_sk_new(OPENSSL_sk_compfunc cmp);
 OPENSSL_STACK *OPENSSL_sk_new_null(void);
 OPENSSL_STACK *OPENSSL_sk_new_reserve(OPENSSL_sk_compfunc c, int n);
 OPENSSL_STACK *OPENSSL_sk_set_thunks(OPENSSL_STACK *st, OPENSSL_sk_freefunc_thunk f_thunk);
+OPENSSL_STACK *OPENSSL_sk_set_cmp_thunks(OPENSSL_STACK *st, int (*c_thunk)(int (*)(const void *, const void *), const void *, const void *));
 int OPENSSL_sk_reserve(OPENSSL_STACK *st, int n);
 void OPENSSL_sk_free(OPENSSL_STACK *);
 void OPENSSL_sk_pop_free(OPENSSL_STACK *st, OPENSSL_sk_freefunc func);
index 81cda111b7845d189c5fbf36216863d3a0773145..275f01301e7a68a13e69c405c1ecf0867de461e9 100644 (file)
@@ -5702,3 +5702,4 @@ EVP_SIGNATURE_has_message_update        ? 4_0_0   EXIST::FUNCTION:
 EVP_MD_CTX_serialize                    ?      4_0_0   EXIST::FUNCTION:
 EVP_MD_CTX_deserialize                  ?      4_0_0   EXIST::FUNCTION:
 OSSL_ENCODER_CTX_ctrl_string            ?      4_0_0   EXIST::FUNCTION:
+OPENSSL_sk_set_cmp_thunks               ?      4_0_0   EXIST::FUNCTION:
index 0b8482e003cf63d64e2b9d0916c8bba1169ce6cc..0c7c1297057a76860eaa1affdeda6c44fc6aa0f7 100644 (file)
@@ -28,9 +28,9 @@ sub generate_stack_macros_int {
 SKM_DEFINE_STACK_OF_INTERNAL(${nametype}, ${realtype}, ${plaintype})
 #define sk_${nametype}_num(sk) OPENSSL_sk_num(ossl_check_const_${nametype}_sk_type(sk))
 #define sk_${nametype}_value(sk, idx) ((${realtype} *)OPENSSL_sk_value(ossl_check_const_${nametype}_sk_type(sk), (idx)))
-#define sk_${nametype}_new(cmp) ((STACK_OF(${nametype}) *)OPENSSL_sk_new(ossl_check_${nametype}_compfunc_type(cmp)))
+#define sk_${nametype}_new(cmp) ((STACK_OF(${nametype}) *)OPENSSL_sk_set_cmp_thunks(OPENSSL_sk_new(ossl_check_${nametype}_compfunc_type(cmp)), sk_${nametype}_cmpfunc_thunk))
 #define sk_${nametype}_new_null() ((STACK_OF(${nametype}) *)OPENSSL_sk_set_thunks(OPENSSL_sk_new_null(), sk_${nametype}_freefunc_thunk))
-#define sk_${nametype}_new_reserve(cmp, n) ((STACK_OF(${nametype}) *)OPENSSL_sk_new_reserve(ossl_check_${nametype}_compfunc_type(cmp), (n)))
+#define sk_${nametype}_new_reserve(cmp, n) ((STACK_OF(${nametype}) *)OPENSSL_sk_set_cmp_thunks(OPENSSL_sk_new_reserve(ossl_check_${nametype}_compfunc_type(cmp), (n)), sk_${nametype}_cmpfunc_thunk))
 #define sk_${nametype}_reserve(sk, n) OPENSSL_sk_reserve(ossl_check_${nametype}_sk_type(sk), (n))
 #define sk_${nametype}_free(sk) OPENSSL_sk_free(ossl_check_${nametype}_sk_type(sk))
 #define sk_${nametype}_zero(sk) OPENSSL_sk_zero(ossl_check_${nametype}_sk_type(sk))