Add malloc failure tests.
This commit fixes a number of crashes caused by malloc failures. They
were found using the -malloc-test=0 option to runner.go which runs tests
many times, causing a different allocation call to fail in each case.
(This test only works on Linux and only looks for crashes caused by
allocation failures, not memory leaks or other errors.)
This is not the complete set of crashes! More can be found by collecting
core dumps from running with -malloc-test=0.
Change-Id: Ia61d19f51e373bccb7bc604642c51e043a74bd83
Reviewed-on: https://boringssl-review.googlesource.com/2320
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/test/CMakeLists.txt b/ssl/test/CMakeLists.txt
index 27d9596..f7f0efc 100644
--- a/ssl/test/CMakeLists.txt
+++ b/ssl/test/CMakeLists.txt
@@ -5,8 +5,9 @@
async_bio.cc
bssl_shim.cc
+ malloc.cc
packeted_bio.cc
test_config.cc
)
-target_link_libraries(bssl_shim ssl crypto)
+target_link_libraries(bssl_shim ssl crypto dl)
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index cdd62ff..9f9fac3 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -41,8 +41,8 @@
static int g_ex_data_index = 0;
-static void SetConfigPtr(SSL *ssl, const TestConfig *config) {
- SSL_set_ex_data(ssl, g_ex_data_index, (void *)config);
+static bool SetConfigPtr(SSL *ssl, const TestConfig *config) {
+ return SSL_set_ex_data(ssl, g_ex_data_index, (void *)config) == 1;
}
static const TestConfig *GetConfigPtr(SSL *ssl) {
@@ -267,7 +267,8 @@
}
dh = DH_get_2048_256(NULL);
- if (!SSL_CTX_set_tmp_dh(ssl_ctx, dh)) {
+ if (dh == NULL ||
+ !SSL_CTX_set_tmp_dh(ssl_ctx, dh)) {
goto err;
}
@@ -335,7 +336,10 @@
return 1;
}
- SetConfigPtr(ssl, config);
+ if (!SetConfigPtr(ssl, config)) {
+ BIO_print_errors_fp(stdout);
+ return 1;
+ }
if (config->fallback_scsv) {
if (!SSL_enable_fallback_scsv(ssl)) {
diff --git a/ssl/test/malloc.cc b/ssl/test/malloc.cc
new file mode 100644
index 0000000..3e3f8d7
--- /dev/null
+++ b/ssl/test/malloc.cc
@@ -0,0 +1,126 @@
+/* Copyright (c) 2014, Google Inc.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY
+ * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
+ * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */
+
+#if defined(__linux__)
+
+#include <stdint.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <unistd.h>
+#include <stdio.h>
+
+#include <new>
+
+
+/* This file defines overrides for the standard allocation functions that allow
+ * a given allocation to be made to fail for testing. If the program is run
+ * with MALLOC_NUMBER_TO_FAIL set to a base-10 number then that allocation will
+ * return NULL. If MALLOC_ABORT_ON_FAIL is also defined then the allocation
+ * will abort() rather than return NULL.
+ *
+ * This code is not thread safe. */
+
+static uint64_t current_malloc_count = 0;
+static uint64_t malloc_number_to_fail = 0;
+static char failure_enabled = 0, abort_on_fail = 0;
+static int in_call = 0;
+
+extern "C" {
+/* These are other names for the standard allocation functions. */
+extern void *__libc_malloc(size_t size);
+extern void *__libc_calloc(size_t num_elems, size_t size);
+extern void *__libc_realloc(void *ptr, size_t size);
+}
+
+static void exit_handler(void) {
+ if (failure_enabled && current_malloc_count > malloc_number_to_fail) {
+ _exit(88);
+ }
+}
+
+static void cpp_new_handler() {
+ // Return to try again. It won't fail a second time.
+ return;
+}
+
+/* should_fail_allocation returns true if the current allocation should fail. */
+static int should_fail_allocation() {
+ static int init = 0;
+ char should_fail;
+
+ if (in_call) {
+ return 0;
+ }
+
+ in_call = 1;
+
+ if (!init) {
+ const char *env = getenv("MALLOC_NUMBER_TO_FAIL");
+ if (env != NULL && env[0] != 0) {
+ char *endptr;
+ malloc_number_to_fail = strtoull(env, &endptr, 10);
+ if (*endptr == 0) {
+ failure_enabled = 1;
+ atexit(exit_handler);
+ std::set_new_handler(cpp_new_handler);
+ }
+ }
+ abort_on_fail = (NULL != getenv("MALLOC_ABORT_ON_FAIL"));
+ init = 1;
+ }
+
+ in_call = 0;
+
+ if (!failure_enabled) {
+ return 0;
+ }
+
+ should_fail = (current_malloc_count == malloc_number_to_fail);
+ current_malloc_count++;
+
+ if (should_fail && abort_on_fail) {
+ abort();
+ }
+ return should_fail;
+}
+
+extern "C" {
+
+void *malloc(size_t size) {
+ if (should_fail_allocation()) {
+ return NULL;
+ }
+
+ return __libc_malloc(size);
+}
+
+void *calloc(size_t num_elems, size_t size) {
+ if (should_fail_allocation()) {
+ return NULL;
+ }
+
+ return __libc_calloc(num_elems, size);
+}
+
+void *realloc(void *ptr, size_t size) {
+ if (should_fail_allocation()) {
+ return NULL;
+ }
+
+ return __libc_realloc(ptr, size);
+}
+
+} // extern "C"
+
+#endif /* defined(linux) */
diff --git a/ssl/test/packeted_bio.cc b/ssl/test/packeted_bio.cc
index 629d6c5..3894d85 100644
--- a/ssl/test/packeted_bio.cc
+++ b/ssl/test/packeted_bio.cc
@@ -67,7 +67,9 @@
uint32_t len = (len_bytes[0] << 24) | (len_bytes[1] << 16) |
(len_bytes[2] << 8) | len_bytes[3];
char *buf = (char *)OPENSSL_malloc(len);
- assert(buf != NULL);
+ if (buf == NULL) {
+ return -1;
+ }
ret = BIO_read(bio->next_bio, buf, len);
assert(ret == (int)len);
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index dca0479..cf1b1f9 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -16,14 +16,19 @@
"os/exec"
"path"
"runtime"
+ "strconv"
"strings"
"sync"
"syscall"
)
-var useValgrind = flag.Bool("valgrind", false, "If true, run code under valgrind")
-var useGDB = flag.Bool("gdb", false, "If true, run BoringSSL code under gdb")
-var flagDebug *bool = flag.Bool("debug", false, "Hexdump the contents of the connection")
+var (
+ useValgrind = flag.Bool("valgrind", false, "If true, run code under valgrind")
+ useGDB = flag.Bool("gdb", false, "If true, run BoringSSL code under gdb")
+ flagDebug *bool = flag.Bool("debug", false, "Hexdump the contents of the connection")
+ mallocTest *int64 = flag.Int64("malloc-test", -1, "If non-negative, run each test with each malloc in turn failing from the given number onwards.")
+ mallocTestDebug *bool = flag.Bool("malloc-test-debug", false, "If true, ask bssl_shim to abort rather than fail a malloc. This can be used with a specific value for --malloc-test to identity the malloc failing that is causing problems.")
+)
const (
rsaCertificateFile = "cert.pem"
@@ -703,7 +708,15 @@
return shimEnd, conn
}
-func runTest(test *testCase, buildDir string) error {
+type moreMallocsError struct{}
+
+func (moreMallocsError) Error() string {
+ return "child process did not exhaust all allocation calls"
+}
+
+var errMoreMallocs = moreMallocsError{}
+
+func runTest(test *testCase, buildDir string, mallocNumToFail int64) error {
if !test.shouldFail && (len(test.expectedError) > 0 || len(test.expectedLocalError) > 0) {
panic("Error expected without shouldFail in " + test.name)
}
@@ -758,6 +771,13 @@
var stdoutBuf, stderrBuf bytes.Buffer
shim.Stdout = &stdoutBuf
shim.Stderr = &stderrBuf
+ if mallocNumToFail >= 0 {
+ shim.Env = []string{"MALLOC_NUMBER_TO_FAIL=" + strconv.FormatInt(mallocNumToFail, 10)}
+ if *mallocTestDebug {
+ shim.Env = append(shim.Env, "MALLOC_ABORT_ON_FAIL=1")
+ }
+ shim.Env = append(shim.Env, "_MALLOC_CHECK=1")
+ }
if err := shim.Start(); err != nil {
panic(err)
@@ -805,6 +825,11 @@
connResume.Close()
childErr := shim.Wait()
+ if exitError, ok := childErr.(*exec.ExitError); ok {
+ if exitError.Sys().(syscall.WaitStatus).ExitStatus() == 88 {
+ return errMoreMallocs
+ }
+ }
stdout := string(stdoutBuf.Bytes())
stderr := string(stderrBuf.Bytes())
@@ -2142,8 +2167,22 @@
defer wg.Done()
for test := range c {
- statusChan <- statusMsg{test: test, started: true}
- err := runTest(test, buildDir)
+ var err error
+
+ if *mallocTest < 0 {
+ statusChan <- statusMsg{test: test, started: true}
+ err = runTest(test, buildDir, -1)
+ } else {
+ for mallocNumToFail := int64(*mallocTest); ; mallocNumToFail++ {
+ statusChan <- statusMsg{test: test, started: true}
+ if err = runTest(test, buildDir, mallocNumToFail); err != errMoreMallocs {
+ if err != nil {
+ fmt.Printf("\n\nmalloc test failed at %d: %s\n", mallocNumToFail, err)
+ }
+ break
+ }
+ }
+ }
statusChan <- statusMsg{test: test, err: err}
}
}
