From: Frederik Wedel-Heinen Date: Sat, 15 Mar 2025 20:02:54 +0000 (+0100) Subject: Adds the concept of thunks to OPENSSL_sk interface X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=21b170df9fd2c6080da119144eac969a940dee38;p=thirdparty%2Fopenssl.git 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 Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/27071) --- diff --git a/CHANGES.md b/CHANGES.md index 8385a97b5a3..28e96ee6707 100644 --- 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 e8139896247..bdf90546886 100644 --- 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 ff2074820f6..0d8a8298ae5 100644 --- 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_sort>() should be called before these find operations. Before OpenSSL 3.3.0 B_push>() returned -1 if I 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 6b36607928f..e50220abebe 100644 --- 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 f0c5c54765a..b31b0254a5d 100644 --- 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 d86007f719f..322c7b42d61 100644 --- 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 7c2459b8a40..b777f15556b 100644 --- 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))