Adds the concept of thunks to OPENSSL_sk interface

This allows applications to call functions of correct signature when free'ing OPENSSL_sk items which UBSan complains about.
Related to #22896.

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/27071)

diff --git a/CHANGES.md b/CHANGES.md
index 8385a97b5a36928082fe592e6d7b91ce700bbabd..28e96ee6707e0e37633992cc6883819f167aee85 100644 (file)
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -12,6 +12,7 @@ appropriate release branch.
 OpenSSL Releases
 ----------------
 
+ - [OpenSSL 3.6](#openssl-36)
  - [OpenSSL 3.5](#openssl-35)
  - [OpenSSL 3.4](#openssl-34)
  - [OpenSSL 3.3](#openssl-33)
@@ -25,16 +26,23 @@ OpenSSL Releases
  - [OpenSSL 1.0.0](#openssl-100)
  - [OpenSSL 0.9.x](#openssl-09x)
 
-OpenSSL 3.5
+OpenSSL 3.6
 -----------
 
 ### Changes between 3.5 and 3.6 [xx XXX xxxx]
 
- * none yet
+ * Support setting a free function thunk to OPENSSL_sk stack types. Using a thunk
+   allows the type specific free function to be called with the correct type
+   information from generic functions like OPENSSL_sk_pop_free().
+
+   *Frederik Wedel-Heinen*
+
+OpenSSL 3.5
+-----------
 
 ### Changes between 3.4 and 3.5 [xx XXX xxxx]
 
-* Added server side support for QUIC
+ * Added server side support for QUIC
 
    *Hugo Landau, Matt Caswell, Tomáš Mráz, Neil Horman, Sasha Nedvedicky, Andrew Dinh*
 

diff --git a/crypto/stack/stack.c b/crypto/stack/stack.c
index e813989624707f6c715dd5d47379f62bd1bd21b6..bdf90546886fda4d75d520c01a8ab8c987740029 100644 (file)
--- a/crypto/stack/stack.c
+++ b/crypto/stack/stack.c
@@ -30,6 +30,7 @@ struct stack_st {
     int sorted;
     int num_alloc;
     OPENSSL_sk_compfunc comp;
+    OPENSSL_sk_freefunc_thunk free_thunk;
 };
 
 OPENSSL_sk_compfunc OPENSSL_sk_set_cmp_func(OPENSSL_STACK *sk,
@@ -255,6 +256,14 @@ int OPENSSL_sk_reserve(OPENSSL_STACK *st, int n)
     return sk_reserve(st, n, 1);
 }
 
+OPENSSL_STACK *OPENSSL_sk_set_thunks(OPENSSL_STACK *st, OPENSSL_sk_freefunc_thunk f_thunk)
+{
+    if (st != NULL)
+        st->free_thunk = f_thunk;
+
+    return st;
+}
+
 int OPENSSL_sk_insert(OPENSSL_STACK *st, const void *data, int loc)
 {
     if (st == NULL) {
@@ -434,9 +443,15 @@ void OPENSSL_sk_pop_free(OPENSSL_STACK *st, OPENSSL_sk_freefunc func)
 
     if (st == NULL)
         return;
-    for (i = 0; i < st->num; i++)
-        if (st->data[i] != NULL)
-            func((char *)st->data[i]);
+
+    for (i = 0; i < st->num; i++) {
+        if (st->data[i] != NULL) {
+            if (st->free_thunk != NULL)
+                st->free_thunk(func, (void *)st->data[i]);
+            else
+                func((void *)st->data[i]);
+        }
+    }
     OPENSSL_sk_free(st);
 }
 

diff --git a/doc/man3/DEFINE_STACK_OF.pod b/doc/man3/DEFINE_STACK_OF.pod
index ff2074820f682326dbd3c403b5396e8c0878365a..0d8a8298ae5577873766571d4c0bee4422551e86 100644 (file)
--- a/doc/man3/DEFINE_STACK_OF.pod
+++ b/doc/man3/DEFINE_STACK_OF.pod
@@ -16,8 +16,8 @@ OPENSSL_sk_dup, OPENSSL_sk_find, OPENSSL_sk_find_ex, OPENSSL_sk_find_all,
 OPENSSL_sk_free, OPENSSL_sk_insert, OPENSSL_sk_is_sorted, OPENSSL_sk_new,
 OPENSSL_sk_new_null, OPENSSL_sk_new_reserve, OPENSSL_sk_num, OPENSSL_sk_pop,
 OPENSSL_sk_pop_free, OPENSSL_sk_push, OPENSSL_sk_reserve, OPENSSL_sk_set,
-OPENSSL_sk_set_cmp_func, OPENSSL_sk_shift, OPENSSL_sk_sort,
-OPENSSL_sk_unshift, OPENSSL_sk_value, OPENSSL_sk_zero
+OPENSSL_sk_set_thunks, OPENSSL_sk_set_cmp_func, OPENSSL_sk_shift,
+OPENSSL_sk_sort, OPENSSL_sk_unshift, OPENSSL_sk_value, OPENSSL_sk_zero
 - stack container
 
 =head1 SYNOPSIS
@@ -241,8 +241,11 @@ OPENSSL_sk_free(), OPENSSL_sk_insert(), OPENSSL_sk_is_sorted(),
 OPENSSL_sk_new(), OPENSSL_sk_new_null(), OPENSSL_sk_new_reserve(),
 OPENSSL_sk_num(), OPENSSL_sk_pop(), OPENSSL_sk_pop_free(), OPENSSL_sk_push(),
 OPENSSL_sk_reserve(), OPENSSL_sk_set(), OPENSSL_sk_set_cmp_func(),
-OPENSSL_sk_shift(), OPENSSL_sk_sort(), OPENSSL_sk_unshift(),
-OPENSSL_sk_value(), OPENSSL_sk_zero().
+OPENSSL_sk_set_thunks(), OPENSSL_sk_shift(), OPENSSL_sk_sort(),
+OPENSSL_sk_unshift(), OPENSSL_sk_value(), OPENSSL_sk_zero().
+
+OPENSSL_sk_set_thunks(), while public by necessity, is actually an internal
+function and should not be used.
 
 =head1 RETURN VALUES
 
@@ -299,6 +302,8 @@ B<sk_I<TYPE>_sort>() should be called before these find operations.
 Before OpenSSL 3.3.0 B<sk_I<TYPE>_push>() returned -1 if I<sk> was NULL. It
 was changed to return 0 in this condition as for other errors.
 
+OPENSSL_sk_set_thunks() was added in OpenSSL 3.6.0.
+
 =head1 COPYRIGHT
 
 Copyright 2000-2024 The OpenSSL Project Authors. All Rights Reserved.

diff --git a/include/openssl/safestack.h.in b/include/openssl/safestack.h.in
index 6b36607928ff86df554e2c29521bf0f6308d7941..e50220abebe97aa6b1dcb2c8439d22b4f4efd4d6 100644 (file)
--- a/include/openssl/safestack.h.in
+++ b/include/openssl/safestack.h.in
@@ -39,6 +39,11 @@ extern "C" {
     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; \
@@ -69,6 +74,11 @@ extern "C" {
     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 int sk_##t1##_num(const STACK_OF(t1) *sk) \
     { \
         return OPENSSL_sk_num((const OPENSSL_STACK *)sk); \
@@ -79,7 +89,11 @@ extern "C" {
     } \
     static ossl_unused ossl_inline STACK_OF(t1) *sk_##t1##_new(sk_##t1##_compfunc compare) \
     { \
-        return (STACK_OF(t1) *)OPENSSL_sk_new((OPENSSL_sk_compfunc)compare); \
+        OPENSSL_STACK *ret = OPENSSL_sk_new((OPENSSL_sk_compfunc)compare); \
+        OPENSSL_sk_freefunc_thunk f_thunk; \
+        \
+        f_thunk = (OPENSSL_sk_freefunc_thunk)sk_##t1##_freefunc_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) \
     { \
@@ -87,7 +101,11 @@ extern "C" {
     } \
     static ossl_unused ossl_inline STACK_OF(t1) *sk_##t1##_new_reserve(sk_##t1##_compfunc compare, int n) \
     { \
-        return (STACK_OF(t1) *)OPENSSL_sk_new_reserve((OPENSSL_sk_compfunc)compare, n); \
+        OPENSSL_STACK *ret = OPENSSL_sk_new_reserve((OPENSSL_sk_compfunc)compare, n); \
+        OPENSSL_sk_freefunc_thunk f_thunk; \
+        \
+        f_thunk = (OPENSSL_sk_freefunc_thunk)sk_##t1##_freefunc_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) \
     { \
@@ -128,6 +146,11 @@ extern "C" {
     } \
     static ossl_unused ossl_inline void sk_##t1##_pop_free(STACK_OF(t1) *sk, sk_##t1##_freefunc freefunc) \
     { \
+        OPENSSL_sk_freefunc_thunk f_thunk; \
+        \
+        f_thunk = (OPENSSL_sk_freefunc_thunk)sk_##t1##_freefunc_thunk; \
+        sk = (STACK_OF(t1) *)OPENSSL_sk_set_thunks((OPENSSL_STACK *)sk, f_thunk); \
+        \
         OPENSSL_sk_pop_free((OPENSSL_STACK *)sk, (OPENSSL_sk_freefunc)freefunc); \
     } \
     static ossl_unused ossl_inline int sk_##t1##_insert(STACK_OF(t1) *sk, t2 *ptr, int idx) \

diff --git a/include/openssl/stack.h b/include/openssl/stack.h
index f0c5c54765af5b6a5197d24ef02b0cdf9d24b3de..b31b0254a5d13b98d133e19851aaec9f9d5ec34b 100644 (file)
--- a/include/openssl/stack.h
+++ b/include/openssl/stack.h
@@ -24,6 +24,7 @@ typedef struct stack_st OPENSSL_STACK; /* Use STACK_OF(...) instead */
 
 typedef int (*OPENSSL_sk_compfunc)(const void *, const void *);
 typedef void (*OPENSSL_sk_freefunc)(void *);
+typedef void (*OPENSSL_sk_freefunc_thunk)(OPENSSL_sk_freefunc, void *);
 typedef void *(*OPENSSL_sk_copyfunc)(const void *);
 
 int OPENSSL_sk_num(const OPENSSL_STACK *);
@@ -34,9 +35,10 @@ void *OPENSSL_sk_set(OPENSSL_STACK *st, int i, const void *data);
 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);
 int OPENSSL_sk_reserve(OPENSSL_STACK *st, int n);
 void OPENSSL_sk_free(OPENSSL_STACK *);
-void OPENSSL_sk_pop_free(OPENSSL_STACK *st, void (*func) (void *));
+void OPENSSL_sk_pop_free(OPENSSL_STACK *st, OPENSSL_sk_freefunc func);
 OPENSSL_STACK *OPENSSL_sk_deep_copy(const OPENSSL_STACK *,
                                     OPENSSL_sk_copyfunc c,
                                     OPENSSL_sk_freefunc f);

diff --git a/util/libcrypto.num b/util/libcrypto.num
index d86007f719fcb8a40b70d52eb445821317f0811f..322c7b42d61d4e3d9e2b3382b35471d4008ce999 100644 (file)
--- a/util/libcrypto.num
+++ b/util/libcrypto.num
@@ -5924,3 +5924,4 @@ OSSL_AA_DIST_POINT_free                 ? 3_5_0   EXIST::FUNCTION:
 OSSL_AA_DIST_POINT_new                  ?      3_5_0   EXIST::FUNCTION:
 OSSL_AA_DIST_POINT_it                   ?      3_5_0   EXIST::FUNCTION:
 PEM_ASN1_write_bio_ctx                  ?      3_5_0   EXIST::FUNCTION:
+OPENSSL_sk_set_thunks                   ?      3_6_0   EXIST::FUNCTION:

diff --git a/util/perl/OpenSSL/stackhash.pm b/util/perl/OpenSSL/stackhash.pm
index 7c2459b8a40612d9a69058ced80d301ff54ef9f6..b777f15556be5c06277a812ad12f87cc39e8bc4b 100644 (file)
--- a/util/perl/OpenSSL/stackhash.pm
+++ b/util/perl/OpenSSL/stackhash.pm
@@ -40,7 +40,7 @@ SKM_DEFINE_STACK_OF_INTERNAL(${nametype}, ${realtype}, ${plaintype})
 #define sk_${nametype}_unshift(sk, ptr) OPENSSL_sk_unshift(ossl_check_${nametype}_sk_type(sk), ossl_check_${nametype}_type(ptr))
 #define sk_${nametype}_pop(sk) ((${realtype} *)OPENSSL_sk_pop(ossl_check_${nametype}_sk_type(sk)))
 #define sk_${nametype}_shift(sk) ((${realtype} *)OPENSSL_sk_shift(ossl_check_${nametype}_sk_type(sk)))
-#define sk_${nametype}_pop_free(sk, freefunc) OPENSSL_sk_pop_free(ossl_check_${nametype}_sk_type(sk),ossl_check_${nametype}_freefunc_type(freefunc))
+#define sk_${nametype}_pop_free(sk, freefunc) OPENSSL_sk_pop_free(ossl_check_${nametype}_sk_type(sk), ossl_check_${nametype}_freefunc_type(freefunc))
 #define sk_${nametype}_insert(sk, ptr, idx) OPENSSL_sk_insert(ossl_check_${nametype}_sk_type(sk), ossl_check_${nametype}_type(ptr), (idx))
 #define sk_${nametype}_set(sk, idx, ptr) ((${realtype} *)OPENSSL_sk_set(ossl_check_${nametype}_sk_type(sk), (idx), ossl_check_${nametype}_type(ptr)))
 #define sk_${nametype}_find(sk, ptr) OPENSSL_sk_find(ossl_check_${nametype}_sk_type(sk), ossl_check_${nametype}_type(ptr))