Merge pull request #5398 from libgit2/pks/valgrind-openssl openssl: fix Valgrind issues in nightly builds
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 8765a97..7f001d3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -58,7 +58,6 @@ OPTION(USE_SHA1 "Enable SHA1. Can be set to CollisionDetection(ON)/HTTPS/Gene
OPTION(USE_GSSAPI "Link with libgssapi for SPNEGO auth" OFF)
OPTION(USE_STANDALONE_FUZZERS "Enable standalone fuzzers (compatible with gcc)" OFF)
OPTION(USE_LEAK_CHECKER "Run tests with leak checker" OFF)
-OPTION(VALGRIND "Configure build for valgrind" OFF)
OPTION(DEBUG_POOL "Enable debug pool allocator" OFF)
OPTION(ENABLE_WERROR "Enable compilation with -Werror" OFF)
OPTION(USE_BUNDLED_ZLIB "Use the bundled version of zlib" OFF)
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index 2b593dd..2575f47 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -19,7 +19,7 @@ jobs:
environmentVariables: |
CC=gcc
CMAKE_GENERATOR=Ninja
- CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DREGEX_BACKEND=builtin -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind -DVALGRIND=on -DUSE_GSSAPI=ON
+ CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DREGEX_BACKEND=builtin -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind -DUSE_GSSAPI=ON
GITTEST_NEGOTIATE_PASSWORD=${{ variables.GITTEST_NEGOTIATE_PASSWORD }}
- job: linux_amd64_xenial_gcc_mbedtls
@@ -35,7 +35,7 @@ jobs:
environmentVariables: |
CC=gcc
CMAKE_GENERATOR=Ninja
- CMAKE_OPTIONS=-DUSE_HTTPS=mbedTLS -DUSE_SHA1=HTTPS -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind -DVALGRIND=on -DUSE_GSSAPI=ON
+ CMAKE_OPTIONS=-DUSE_HTTPS=mbedTLS -DUSE_SHA1=HTTPS -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind -DUSE_GSSAPI=ON
GITTEST_NEGOTIATE_PASSWORD=${{ variables.GITTEST_NEGOTIATE_PASSWORD }}
- job: linux_amd64_xenial_clang_openssl
@@ -51,7 +51,7 @@ jobs:
environmentVariables: |
CC=clang
CMAKE_GENERATOR=Ninja
- CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind -DVALGRIND=on -DUSE_GSSAPI=ON
+ CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind -DUSE_GSSAPI=ON
GITTEST_NEGOTIATE_PASSWORD=${{ variables.GITTEST_NEGOTIATE_PASSWORD }}
- job: linux_amd64_xenial_clang_mbedtls
@@ -67,7 +67,7 @@ jobs:
environmentVariables: |
CC=clang
CMAKE_GENERATOR=Ninja
- CMAKE_OPTIONS=-DUSE_HTTPS=mbedTLS -DUSE_SHA1=HTTPS -DREGEX_BACKEND=pcre -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind -DVALGRIND=on -DUSE_GSSAPI=ON
+ CMAKE_OPTIONS=-DUSE_HTTPS=mbedTLS -DUSE_SHA1=HTTPS -DREGEX_BACKEND=pcre -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind -DUSE_GSSAPI=ON
GITTEST_NEGOTIATE_PASSWORD=${{ variables.GITTEST_NEGOTIATE_PASSWORD }}
- job: macos
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 9e4a41a..bcc6a01 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -288,14 +288,13 @@ IF (WIN32 AND NOT CYGWIN)
ELSEIF (AMIGA)
ADD_DEFINITIONS(-DNO_ADDRINFO -DNO_READDIR_R -DNO_MMAP)
ELSE()
- ADD_FEATURE_INFO(valgrind VALGRIND "valgrind hints")
- IF (VALGRIND)
- ADD_DEFINITIONS(-DVALGRIND)
- ENDIF()
-
FILE(GLOB SRC_OS unix/*.c unix/*.h)
ENDIF()
+IF (USE_LEAK_CHECKER STREQUAL "valgrind")
+ ADD_DEFINITIONS(-DVALGRIND)
+ENDIF()
+
FILE(GLOB SRC_GIT2 *.c *.h
allocators/*.c allocators/*.h
streams/*.c streams/*.h
diff --git a/src/streams/openssl.c b/src/streams/openssl.c
index 98a3635..458810c 100644
--- a/src/streams/openssl.c
+++ b/src/streams/openssl.c
@@ -30,10 +30,6 @@
#include <openssl/x509v3.h>
#include <openssl/bio.h>
-#ifdef VALGRIND
-# include <valgrind/memcheck.h>
-#endif
-
SSL_CTX *git__ssl_ctx;
#define GIT_SSL_DEFAULT_CIPHERS "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-DSS-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:DHE-DSS-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES256-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA:DHE-DSS-AES128-SHA256:DHE-DSS-AES256-SHA256:DHE-DSS-AES128-SHA:DHE-DSS-AES256-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA"
@@ -200,16 +196,69 @@ static void shutdown_ssl(void)
}
}
+#ifdef VALGRIND
+#ifdef OPENSSL_LEGACY_API
+static void *git_openssl_malloc(size_t bytes)
+{
+ return git__calloc(1, bytes);
+}
+
+static void *git_openssl_realloc(void *mem, size_t size)
+{
+ return git__realloc(mem, size);
+}
+
+static void git_openssl_free(void *mem)
+{
+ return git__free(mem);
+}
+#else
+static void *git_openssl_malloc(size_t bytes, const char *file, int line)
+{
+ GIT_UNUSED(file);
+ GIT_UNUSED(line);
+ return git__calloc(1, bytes);
+}
+
+static void *git_openssl_realloc(void *mem, size_t size, const char *file, int line)
+{
+ GIT_UNUSED(file);
+ GIT_UNUSED(line);
+ return git__realloc(mem, size);
+}
+
+static void git_openssl_free(void *mem, const char *file, int line)
+{
+ GIT_UNUSED(file);
+ GIT_UNUSED(line);
+ return git__free(mem);
+}
+#endif
+#endif
+
int git_openssl_stream_global_init(void)
{
long ssl_opts = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3;
const char *ciphers = git_libgit2__ssl_ciphers();
+#ifdef VALGRIND
+ static bool allocators_initialized = false;
+#endif
/* Older OpenSSL and MacOS OpenSSL doesn't have this */
#ifdef SSL_OP_NO_COMPRESSION
ssl_opts |= SSL_OP_NO_COMPRESSION;
#endif
+#ifdef VALGRIND
+ /* Swap in our own allocator functions that initialize allocated memory */
+ if (!allocators_initialized &&
+ CRYPTO_set_mem_functions(git_openssl_malloc,
+ git_openssl_realloc,
+ git_openssl_free) != 1)
+ goto error;
+ allocators_initialized = true;
+#endif
+
OPENSSL_init_ssl(0, NULL);
/*
@@ -314,11 +363,6 @@ static int bio_read(BIO *b, char *buf, int len)
static int bio_write(BIO *b, const char *buf, int len)
{
git_stream *io = (git_stream *) BIO_get_data(b);
-
-#ifdef VALGRIND
- VALGRIND_MAKE_MEM_DEFINED(buf, len);
-#endif
-
return (int) git_stream_write(io, buf, len, 0);
}
@@ -595,10 +639,6 @@ static int openssl_connect(git_stream *stream)
BIO_set_data(bio, st->io);
SSL_set_bio(st->ssl, bio, bio);
-#ifdef VALGRIND
- VALGRIND_MAKE_MEM_DEFINED(st->ssl, sizeof(SSL));
-#endif
-
/* specify the host in case SNI is needed */
#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
SSL_set_tlsext_host_name(st->ssl, st->host);
@@ -609,10 +649,6 @@ static int openssl_connect(git_stream *stream)
st->connected = true;
-#ifdef VALGRIND
- VALGRIND_MAKE_MEM_DEFINED(st->ssl, sizeof(SSL));
-#endif
-
return verify_server_cert(st->ssl, st->host);
}
@@ -679,10 +715,6 @@ static ssize_t openssl_read(git_stream *stream, void *data, size_t len)
if ((ret = SSL_read(st->ssl, data, len)) <= 0)
return ssl_set_error(st->ssl, ret);
-#ifdef VALGRIND
- VALGRIND_MAKE_MEM_DEFINED(data, ret);
-#endif
-
return ret;
}