Guard use of sdallocx with BORINGSSL_SDALLOCX
See comment in change and https://github.com/grpc/grpc/issues/25450
Update-note: consumers may wish to define BORINGSSL_SDALLOCX if using
tcmalloc.
Change-Id: I123fe31a6c4013f1ce0c056f82a316c71df84939
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48885
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
diff --git a/crypto/mem.c b/crypto/mem.c
index 883439b..cc764cc 100644
--- a/crypto/mem.c
+++ b/crypto/mem.c
@@ -93,15 +93,19 @@
#define WEAK_SYMBOL_FUNC(rettype, name, args) static rettype(*name) args = NULL;
#endif
+#if defined(BORINGSSL_SDALLOCX)
// sdallocx is a sized |free| function. By passing the size (which we happen to
-// always know in BoringSSL), the malloc implementation can save work. We cannot
-// depend on |sdallocx| being available, however, so it's a weak symbol.
+// always know in BoringSSL), the malloc implementation can save work.
//
-// This will always be safe, but will only be overridden if the malloc
-// implementation is statically linked with BoringSSL. So, if |sdallocx| is
-// provided in, say, libc.so, we still won't use it because that's dynamically
-// linked. This isn't an ideal result, but its helps in some cases.
-WEAK_SYMBOL_FUNC(void, sdallocx, (void *ptr, size_t size, int flags));
+// This is guarded by BORINGSSL_SDALLOCX, rather than being a weak symbol,
+// because it can work poorly if there are two malloc implementations in the
+// address space. (Which probably isn't valid, ODR etc, but
+// https://github.com/grpc/grpc/issues/25450). In that situation, |malloc| can
+// come from one allocator but |sdallocx| from another and crashes quickly
+// result. We can't match |sdallocx| with |mallocx| because tcmalloc only
+// provides the former, so a mismatch can still happen.
+void sdallocx(void *ptr, size_t size, int flags);
+#endif
// The following three functions can be defined to override default heap
// allocation and freeing. If defined, it is the responsibility of
@@ -162,11 +166,11 @@
size_t size = *(size_t *)ptr;
OPENSSL_cleanse(ptr, size + OPENSSL_MALLOC_PREFIX);
- if (sdallocx) {
- sdallocx(ptr, size + OPENSSL_MALLOC_PREFIX, 0 /* flags */);
- } else {
- free(ptr);
- }
+#if defined(BORINGSSL_SDALLOCX)
+ sdallocx(ptr, size + OPENSSL_MALLOC_PREFIX, 0 /* flags */);
+#else
+ free(ptr);
+#endif
}
void *OPENSSL_realloc(void *orig_ptr, size_t new_size) {