diff --git a/ChangeLog b/ChangeLog index bb3af66b7..def0f6e97 100644 --- a/ChangeLog +++ b/ChangeLog @@ -36,6 +36,8 @@ Bugfix * Fix issue that caused a crash if invalid curves were passed to mbedtls_ssl_conf_curves. #373 * Fix issue in ssl_fork_server which was preventing it from functioning. #429 + * Fix memory leaks in test framework + * Fix test in ssl-opt.sh that does not run properly with valgrind Changes * On ARM platforms, when compiling with -O0 with GCC, Clang or armcc5, diff --git a/library/rsa.c b/library/rsa.c index 8e17ac4fd..e7bc0ccab 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -804,7 +804,7 @@ int mbedtls_rsa_rsaes_pkcs1_v15_decrypt( mbedtls_rsa_context *ctx, int ret; size_t ilen, pad_count = 0, i; unsigned char *p, bad, pad_done = 0; -#ifdef __clang_analyzer__ +#if defined(__clang_analyzer__) /* Shut up Clang, mbedtls_rsa_public/private writes to this */ unsigned char buf[MBEDTLS_MPI_MAX_SIZE] = { }; #else @@ -1189,7 +1189,7 @@ int mbedtls_rsa_rsassa_pss_verify_ext( mbedtls_rsa_context *ctx, size_t slen, msb; const mbedtls_md_info_t *md_info; mbedtls_md_context_t md_ctx; -#ifdef __clang_analyzer__ +#if defined(__clang_analyzer__) /* Shut up Clang, mbedtls_rsa_public/private writes to this */ unsigned char buf[MBEDTLS_MPI_MAX_SIZE] = { }; #else @@ -1336,7 +1336,7 @@ int mbedtls_rsa_rsassa_pkcs1_v15_verify( mbedtls_rsa_context *ctx, mbedtls_md_type_t msg_md_alg; const mbedtls_md_info_t *md_info; mbedtls_asn1_buf oid; -#ifdef __clang_analyzer__ +#if defined(__clang_analyzer__) /* Shut up Clang, mbedtls_rsa_public/private writes to this */ unsigned char buf[MBEDTLS_MPI_MAX_SIZE] = { }; #else diff --git a/programs/hash/generic_sum.c b/programs/hash/generic_sum.c index 7805a79bc..d1e81d491 100644 --- a/programs/hash/generic_sum.c +++ b/programs/hash/generic_sum.c @@ -83,8 +83,13 @@ static int generic_check( const mbedtls_md_info_t *md_info, char *filename ) int nb_err1, nb_err2; int nb_tot1, nb_tot2; unsigned char sum[MBEDTLS_MD_MAX_SIZE]; - char buf[MBEDTLS_MD_MAX_SIZE * 2 + 1] = { }, line[1024]; + char line[1024]; char diff; +#if defined(__clang_analyzer__) + char buf[MBEDTLS_MD_MAX_SIZE * 2 + 1] = { }; +#else + char buf[MBEDTLS_MD_MAX_SIZE * 2 + 1]; +#endif if( ( f = fopen( filename, "rb" ) ) == NULL ) { diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index c08af7b04..536add274 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -130,6 +130,13 @@ not_with_valgrind() { fi } +# skip the next test if valgrind is NOT in use +only_with_valgrind() { + if [ "$MEMCHECK" -eq 0 ]; then + SKIP_NEXT="YES" + fi +} + # multiply the client timeout delay by the given factor for the next test needs_more_time() { CLI_DELAY_FACTOR=$1 @@ -408,32 +415,33 @@ run_test() { # check other assertions # lines beginning with == are added by valgrind, ignore them + # lines with 'Serious error when reading debug info', are valgrind issues as well while [ $# -gt 0 ] do case $1 in "-s") - if grep -v '^==' $SRV_OUT | grep "$2" >/dev/null; then :; else + if grep -v '^==' $SRV_OUT | grep -v 'Serious error when reading debug info' | grep "$2" >/dev/null; then :; else fail "-s $2" return fi ;; "-c") - if grep -v '^==' $CLI_OUT | grep "$2" >/dev/null; then :; else + if grep -v '^==' $CLI_OUT | grep -v 'Serious error when reading debug info' | grep "$2" >/dev/null; then :; else fail "-c $2" return fi ;; "-S") - if grep -v '^==' $SRV_OUT | grep "$2" >/dev/null; then + if grep -v '^==' $SRV_OUT | grep -v 'Serious error when reading debug info' | grep "$2" >/dev/null; then fail "-S $2" return fi ;; "-C") - if grep -v '^==' $CLI_OUT | grep "$2" >/dev/null; then + if grep -v '^==' $CLI_OUT | grep -v 'Serious error when reading debug info' | grep "$2" >/dev/null; then fail "-C $2" return fi @@ -3048,13 +3056,22 @@ run_test "DTLS client reconnect from same port: reconnect" \ -S "The operation timed out" \ -s "Client initiated reconnection from same port" -run_test "DTLS client reconnect from same port: reconnect, nbio" \ +not_with_valgrind # server/client too slow to respond in time (next test has higher timeouts) +run_test "DTLS client reconnect from same port: reconnect, nbio, no valgrind" \ "$P_SRV dtls=1 exchanges=2 read_timeout=1000 nbio=2" \ "$P_CLI dtls=1 exchanges=2 debug_level=2 hs_timeout=500-1000 reconnect_hard=1" \ 0 \ -S "The operation timed out" \ -s "Client initiated reconnection from same port" +only_with_valgrind # Only with valgrind, do previous test but with higher read_timeout and hs_timeout +run_test "DTLS client reconnect from same port: reconnect, nbio, valgrind" \ + "$P_SRV dtls=1 exchanges=2 read_timeout=2000 nbio=2 hs_timeout=1500-6000" \ + "$P_CLI dtls=1 exchanges=2 debug_level=2 hs_timeout=1500-3000 reconnect_hard=1" \ + 0 \ + -S "The operation timed out" \ + -s "Client initiated reconnection from same port" + run_test "DTLS client reconnect from same port: no cookies" \ "$P_SRV dtls=1 exchanges=2 read_timeout=1000 cookies=0" \ "$P_CLI dtls=1 exchanges=2 debug_level=2 hs_timeout=500-8000 reconnect_hard=1" \ diff --git a/tests/suites/main_test.function b/tests/suites/main_test.function index c5d6cd86b..f18248578 100644 --- a/tests/suites/main_test.function +++ b/tests/suites/main_test.function @@ -321,6 +321,9 @@ int main(int argc, const char *argv[]) testfile_index < testfile_count; testfile_index++ ) { + int unmet_dep_count = 0; + char *unmet_dependencies[20]; + test_filename = test_files[ testfile_index ]; file = fopen( test_filename, "r" ); @@ -333,8 +336,12 @@ int main(int argc, const char *argv[]) while( !feof( file ) ) { - int unmet_dep_count = 0; - char *unmet_dependencies[20]; + if( unmet_dep_count > 0 ) + { + mbedtls_printf("FATAL: Dep count larger than zero at start of loop\n"); + mbedtls_exit( MBEDTLS_EXIT_FAILURE ); + } + unmet_dep_count = 0; if( ( ret = get_line( file, buf, sizeof(buf) ) ) != 0 ) break; @@ -357,8 +364,15 @@ int main(int argc, const char *argv[]) { if( dep_check( params[i] ) != DEPENDENCY_SUPPORTED ) { - unmet_dependencies[ i-1 ] = strdup(params[i]); - if( unmet_dependencies[ i-1 ] == NULL ) + if( 0 == option_verbose ) + { + /* Only one count is needed if not verbose */ + unmet_dep_count++; + break; + } + + unmet_dependencies[ unmet_dep_count ] = strdup(params[i]); + if( unmet_dependencies[ unmet_dep_count ] == NULL ) { mbedtls_printf("FATAL: Out of memory\n"); mbedtls_exit( MBEDTLS_EXIT_FAILURE ); @@ -392,16 +406,17 @@ int main(int argc, const char *argv[]) if( 1 == option_verbose && unmet_dep_count > 0 ) { mbedtls_fprintf( stdout, " Unmet dependencies: " ); - while( unmet_dep_count > 0) + for( i = 0; i < unmet_dep_count; i++ ) { mbedtls_fprintf(stdout, "%s ", - unmet_dependencies[unmet_dep_count - 1]); - free(unmet_dependencies[unmet_dep_count - 1]); - unmet_dep_count--; + unmet_dependencies[i]); + free(unmet_dependencies[i]); } mbedtls_fprintf( stdout, "\n" ); } fflush( stdout ); + + unmet_dep_count = 0; } else if( ret == DISPATCH_TEST_SUCCESS && test_errors == 0 ) { @@ -427,6 +442,10 @@ int main(int argc, const char *argv[]) } } fclose(file); + + /* In case we encounter early end of file */ + for( i = 0; i < unmet_dep_count; i++ ) + free( unmet_dependencies[i] ); } mbedtls_fprintf( stdout, "\n----------------------------------------------------------------------------\n\n");