kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix undefined casts in sk_*_pop_free and sk_*_deep_copy.

Browse Source 
Unfortunately, some projects are calling into sk_pop_free directly, so
we must leave a compatibility version around for now.

Bug: chromium:785442
Change-Id: I1577fce6f23af02114f7e9f7bf2b14e9d22fa9ae
Reviewed-on: https://boringssl-review.googlesource.com/32113
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
David Benjamin 2018-09-23 15:59:51 -05:00 committed by CQ bot account: commit-bot@chromium.org
parent cbc3e076fc
commit fb4e2e0f0c
2 changed files with 81 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
crypto/stack/stack.c
View File

@ -133,19 +133,31 @@ void sk_free(_STACK *sk) {
OPENSSL_free(sk); OPENSSL_free(sk);
} }
void sk_pop_free(_STACK *sk, void (*func)(void *)) { void sk_pop_free_ex(_STACK *sk, void (*call_free_func)(stack_free_func, void *),
stack_free_func free_func) {
if (sk == NULL) { if (sk == NULL) {
return; return;
} }
for (size_t i = 0; i < sk->num; i++) { for (size_t i = 0; i < sk->num; i++) {
if (sk->data[i] != NULL) { if (sk->data[i] != NULL) {
func(sk->data[i]); call_free_func(free_func, sk->data[i]);
} }
} }
sk_free(sk); sk_free(sk);
} }
// Historically, |sk_pop_free| called the function as |stack_free_func|
// directly. This is undefined in C. Some callers called |sk_pop_free| directly,
// so we must maintain a compatibility version for now.
static void call_free_func_legacy(stack_free_func func, void *ptr) {
func(ptr);
}
void sk_pop_free(_STACK *sk, stack_free_func free_func) {
sk_pop_free_ex(sk, call_free_func_legacy, free_func);
}
size_t sk_insert(_STACK *sk, void *p, size_t where) { size_t sk_insert(_STACK *sk, void *p, size_t where) {
if (sk == NULL) { if (sk == NULL) {
return 0; return 0;
@ -363,8 +375,11 @@ stack_cmp_func sk_set_cmp_func(_STACK *sk, stack_cmp_func comp) {
return old; return old;
} }
_STACK *sk_deep_copy(const _STACK *sk, void *(*copy_func)(void *), _STACK *sk_deep_copy(const _STACK *sk,
void (*free_func)(void *)) { void *(*call_copy_func)(stack_copy_func, void *),
stack_copy_func copy_func,
void (*call_free_func)(stack_free_func, void *),
stack_free_func free_func) {
_STACK *ret = sk_dup(sk); _STACK *ret = sk_dup(sk);
if (ret == NULL) { if (ret == NULL) {
return NULL; return NULL;
@ -374,11 +389,11 @@ _STACK *sk_deep_copy(const _STACK *sk, void *(*copy_func)(void *),
if (ret->data[i] == NULL) { if (ret->data[i] == NULL) {
continue; continue;
} }
ret->data[i] = copy_func(ret->data[i]); ret->data[i] = call_copy_func(copy_func, ret->data[i]);
if (ret->data[i] == NULL) { if (ret->data[i] == NULL) {
for (size_t j = 0; j < i; j++) { for (size_t j = 0; j < i; j++) {
if (ret->data[j] != NULL) { if (ret->data[j] != NULL) {
free_func(ret->data[j]); call_free_func(free_func, ret->data[j]);
} }
} }
sk_free(ret); sk_free(ret);

76
include/openssl/stack.h
View File

@ -86,6 +86,16 @@ extern "C" {
// STACK_OF(FOO), the macros would be sk_FOO_new, sk_FOO_pop etc. // STACK_OF(FOO), the macros would be sk_FOO_new, sk_FOO_pop etc.
// stack_free_func is a function that frees an element in a stack. Note its
// actual type is void (*)(T *) for some T. Low-level |sk_*| functions will be
// passed a type-specific wrapper to call it correctly.
typedef void (*stack_free_func)(void *ptr);
// stack_copy_func is a function that copies an element in a stack. Note its
// actual type is T *(*)(T *) for some T. Low-level |sk_*| functions will be
// passed a type-specific wrapper to call it correctly.
typedef void *(*stack_copy_func)(void *ptr);
// stack_cmp_func is a comparison function that returns a value < 0, 0 or > 0 // stack_cmp_func is a comparison function that returns a value < 0, 0 or > 0
// if |*a| is less than, equal to or greater than |*b|, respectively. Note the // if |*a| is less than, equal to or greater than |*b|, respectively. Note the
// extra indirection - the function is given a pointer to a pointer to the // extra indirection - the function is given a pointer to a pointer to the
@ -140,12 +150,17 @@ OPENSSL_EXPORT void *sk_value(const _STACK *sk, size_t i);
OPENSSL_EXPORT void *sk_set(_STACK *sk, size_t i, void *p); OPENSSL_EXPORT void *sk_set(_STACK *sk, size_t i, void *p);
// sk_free frees the given stack and array of pointers, but does nothing to // sk_free frees the given stack and array of pointers, but does nothing to
// free the individual elements. Also see |sk_pop_free|. // free the individual elements. Also see |sk_pop_free_ex|.
OPENSSL_EXPORT void sk_free(_STACK *sk); OPENSSL_EXPORT void sk_free(_STACK *sk);
// sk_pop_free calls |free_func| on each element in the stack and then frees // sk_pop_free_ex calls |free_func| on each element in the stack and then frees
// the stack itself. // the stack itself. Note this corresponds to |sk_FOO_pop_free|. It is named
OPENSSL_EXPORT void sk_pop_free(_STACK *sk, void (*free_func)(void *)); // |sk_pop_free_ex| as a workaround for existing code calling an older version
// of |sk_pop_free|.
OPENSSL_EXPORT void sk_pop_free_ex(_STACK *sk,
void (*call_free_func)(stack_free_func,
void *),
stack_free_func free_func);
// sk_insert inserts |p| into the stack at index |where|, moving existing // sk_insert inserts |p| into the stack at index |where|, moving existing
// elements if needed. It returns the length of the new stack, or zero on // elements if needed. It returns the length of the new stack, or zero on
@ -207,9 +222,20 @@ OPENSSL_EXPORT stack_cmp_func sk_set_cmp_func(_STACK *sk, stack_cmp_func comp);
// sk_deep_copy performs a copy of |sk| and of each of the non-NULL elements in // sk_deep_copy performs a copy of |sk| and of each of the non-NULL elements in
// |sk| by using |copy_func|. If an error occurs, |free_func| is used to free // |sk| by using |copy_func|. If an error occurs, |free_func| is used to free
// any copies already made and NULL is returned. // any copies already made and NULL is returned.
OPENSSL_EXPORT _STACK *sk_deep_copy(const _STACK *sk, OPENSSL_EXPORT _STACK *sk_deep_copy(
void *(*copy_func)(void *), const _STACK *sk, void *(*call_copy_func)(stack_copy_func, void *),
void (*free_func)(void *)); stack_copy_func copy_func, void (*call_free_func)(stack_free_func, void *),
stack_free_func free_func);
// Deprecated functions.
// sk_pop_free behaves like |sk_pop_free_ex| but performs an invalid function
// pointer cast. It exists because some existing callers called |sk_pop_free|
// directly.
//
// TODO(davidben): Migrate callers to bssl::UniquePtr and remove this.
OPENSSL_EXPORT void sk_pop_free(_STACK *sk, stack_free_func free_func);
// Defining stack types. // Defining stack types.
@ -252,8 +278,20 @@ BSSL_NAMESPACE_END
#define BORINGSSL_DEFINE_STACK_OF_IMPL(name, ptrtype, constptrtype) \ #define BORINGSSL_DEFINE_STACK_OF_IMPL(name, ptrtype, constptrtype) \
DECLARE_STACK_OF(name) \ DECLARE_STACK_OF(name) \
\ \
typedef void (*stack_##name##_free_func)(ptrtype); \
typedef ptrtype (*stack_##name##_copy_func)(ptrtype); \
typedef int (*stack_##name##_cmp_func)(constptrtype *a, constptrtype *b); \ typedef int (*stack_##name##_cmp_func)(constptrtype *a, constptrtype *b); \
\ \
static inline OPENSSL_UNUSED void sk_##name##_call_free_func( \
stack_free_func free_func, void *ptr) { \
((stack_##name##_free_func)free_func)((ptrtype)ptr); \
} \
\
static inline OPENSSL_UNUSED void *sk_##name##_call_copy_func( \
stack_copy_func copy_func, void *ptr) { \
return (void *)((stack_##name##_copy_func)copy_func)((ptrtype)ptr); \
} \
\
static inline OPENSSL_UNUSED STACK_OF(name) * \ static inline OPENSSL_UNUSED STACK_OF(name) * \
sk_##name##_new(stack_##name##_cmp_func comp) { \ sk_##name##_new(stack_##name##_cmp_func comp) { \
return (STACK_OF(name) *)sk_new((stack_cmp_func)comp); \ return (STACK_OF(name) *)sk_new((stack_cmp_func)comp); \
@ -287,8 +325,9 @@ BSSL_NAMESPACE_END
} \ } \
\ \
static inline OPENSSL_UNUSED void sk_##name##_pop_free( \ static inline OPENSSL_UNUSED void sk_##name##_pop_free( \
STACK_OF(name) *sk, void (*free_func)(ptrtype p)) { \ STACK_OF(name) *sk, stack_##name##_free_func free_func) { \
sk_pop_free((_STACK *)sk, (void (*)(void *))free_func); \ sk_pop_free_ex((_STACK *)sk, sk_##name##_call_free_func, \
(stack_free_func)free_func); \
} \ } \
\ \
static inline OPENSSL_UNUSED size_t sk_##name##_insert( \ static inline OPENSSL_UNUSED size_t sk_##name##_insert( \
@ -349,9 +388,10 @@ BSSL_NAMESPACE_END
sk_##name##_deep_copy(const STACK_OF(name) *sk, \ sk_##name##_deep_copy(const STACK_OF(name) *sk, \
ptrtype(*copy_func)(ptrtype), \ ptrtype(*copy_func)(ptrtype), \
void (*free_func)(ptrtype)) { \ void (*free_func)(ptrtype)) { \
return (STACK_OF(name) *)sk_deep_copy((const _STACK *)sk, \ return (STACK_OF(name) *)sk_deep_copy( \
(void *(*)(void *))copy_func, \ (const _STACK *)sk, sk_##name##_call_copy_func, \
(void (*)(void *))free_func); \ (stack_copy_func)copy_func, sk_##name##_call_free_func, \
(stack_free_func)free_func); \
} }
// DEFINE_NAMED_STACK_OF defines |STACK_OF(name)| to be a stack whose elements // DEFINE_NAMED_STACK_OF defines |STACK_OF(name)| to be a stack whose elements
@ -410,10 +450,14 @@ template <typename Stack>
struct DeleterImpl< struct DeleterImpl<
Stack, typename std::enable_if<!StackTraits<Stack>::kIsConst>::type> { Stack, typename std::enable_if<!StackTraits<Stack>::kIsConst>::type> {
static void Free(Stack *sk) { static void Free(Stack *sk) {
sk_pop_free( // sk_FOO_pop_free is defined by macros and bound by name, so we cannot
reinterpret_cast<_STACK *>(sk), // access it from C++ here.
reinterpret_cast<void (*)(void *)>( using Type = typename StackTraits<Stack>::Type;
DeleterImpl<typename StackTraits<Stack>::Type>::Free)); sk_pop_free_ex(reinterpret_cast<_STACK *>(sk),
[](stack_free_func unused, void *ptr) {
DeleterImpl<Type>::Free(reinterpret_cast<Type *>(ptr));
},
nullptr);
} }
}; };