From 004206c7f52227f712abd10f0117ac8102c5f35f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 21 Oct 2019 17:11:33 +0200 Subject: [PATCH 1/4] Unify ASan options in make builds Use a common set of options when building with Asan without CMake. --- tests/scripts/all.sh | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index d4cb0111c..5c74c71d1 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -128,6 +128,9 @@ pre_initialize_variables () { # Include more verbose output for failing tests run by CMake export CTEST_OUTPUT_ON_FAILURE=1 + # CFLAGS and LDFLAGS for Asan builds that don't use CMake + ASAN_CFLAGS='-Werror -Wall -Wextra -fsanitize=address,undefined' + # Gather the list of available components. These are the functions # defined in this script whose name starts with "component_". # Parse the script with sed, because in sh there is no way to list @@ -826,7 +829,7 @@ component_test_malloc_0_null () { msg "build: malloc(0) returns NULL (ASan+UBSan build)" scripts/config.pl full scripts/config.pl unset MBEDTLS_MEMORY_BUFFER_ALLOC_C - make CC=gcc CFLAGS="'-DMBEDTLS_CONFIG_FILE=\"$PWD/tests/configs/config-wrapper-malloc-0-null.h\"' -O -Werror -Wall -Wextra -fsanitize=address,undefined" LDFLAGS='-fsanitize=address,undefined' + make CC=gcc CFLAGS="'-DMBEDTLS_CONFIG_FILE=\"$PWD/tests/configs/config-wrapper-malloc-0-null.h\"' $ASAN_CFLAGS -O" LDFLAGS="$ASAN_CFLAGS" msg "test: malloc(0) returns NULL (ASan+UBSan build)" make test @@ -868,7 +871,7 @@ component_test_aes_fewer_tables_and_rom_tables () { component_test_se_default () { msg "build: default config + MBEDTLS_PSA_CRYPTO_SE_C" scripts/config.pl set MBEDTLS_PSA_CRYPTO_SE_C - make CC=clang CFLAGS='-Werror -Wall -Wextra -Wno-unused-function -Os -fsanitize=address' LDFLAGS='-fsanitize=address' + make CC=clang CFLAGS="$ASAN_CFLAGS -Os" LDFLAGS="$ASAN_CFLAGS" msg "test: default config + MBEDTLS_PSA_CRYPTO_SE_C" make test @@ -877,7 +880,7 @@ component_test_se_default () { component_test_se_full () { msg "build: full config + MBEDTLS_PSA_CRYPTO_SE_C" scripts/config.pl set MBEDTLS_PSA_CRYPTO_SE_C - make CC=gcc CFLAGS='-Werror -Wall -Wextra -O2 -fsanitize=address' LDFLAGS='-fsanitize=address' + make CC=gcc CFLAGS="$ASAN_CFLAGS -O2" LDFLAGS="$ASAN_CFLAGS" msg "test: full config + MBEDTLS_PSA_CRYPTO_SE_C" make test @@ -912,7 +915,7 @@ component_test_m32_o0 () { # Build once with -O0, to compile out the i386 specific inline assembly msg "build: i386, make, gcc -O0 (ASan build)" # ~ 30s scripts/config.pl full - make CC=gcc CFLAGS='-O0 -Werror -Wall -Wextra -m32 -fsanitize=address' LDFLAGS='-m32 -fsanitize=address' + make CC=gcc CFLAGS="$ASAN_CFLAGS -m32 -O0" LDFLAGS="-m32 $ASAN_CFLAGS" msg "test: i386, make, gcc -O0 (ASan build)" make test @@ -931,7 +934,7 @@ component_test_m32_o1 () { scripts/config.pl unset MBEDTLS_MEMORY_BACKTRACE scripts/config.pl unset MBEDTLS_MEMORY_BUFFER_ALLOC_C scripts/config.pl unset MBEDTLS_MEMORY_DEBUG - make CC=gcc CFLAGS='-O1 -Werror -Wall -Wextra -m32 -fsanitize=address' LDFLAGS='-m32 -fsanitize=address' + make CC=gcc CFLAGS="$ASAN_CFLAGS -m32 -O1" LDFLAGS="-m32 $ASAN_CFLAGS" msg "test: i386, make, gcc -O1 (ASan build)" make test @@ -944,7 +947,7 @@ component_test_m32_everest () { msg "build: i386, Everest ECDH context (ASan build)" # ~ 6 min scripts/config.pl unset MBEDTLS_ECDH_LEGACY_CONTEXT scripts/config.pl set MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED - make CC=gcc CFLAGS='-O2 -Werror -Wall -Wextra -m32 -fsanitize=address' + make CC=gcc CFLAGS="$ASAN_CFLAGS -m32 -O2" LDFLAGS="-m32 $ASAN_CFLAGS" msg "test: i386, Everest ECDH context - main suites (inc. selftests) (ASan build)" # ~ 50s make test From bfeed663d2e19704596744af8e2cb3378231a06d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 21 Oct 2019 19:06:33 +0200 Subject: [PATCH 2/4] Asan make builds: avoid sanitizer recovery Some sanitizers default to displaying an error message and recovering. This could result in a test being recorded as passing despite a complaint from the sanitizer. Turn off sanitizer recovery to avoid this risk. --- tests/scripts/all.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 5c74c71d1..2414f452a 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -129,7 +129,7 @@ pre_initialize_variables () { export CTEST_OUTPUT_ON_FAILURE=1 # CFLAGS and LDFLAGS for Asan builds that don't use CMake - ASAN_CFLAGS='-Werror -Wall -Wextra -fsanitize=address,undefined' + ASAN_CFLAGS='-Werror -Wall -Wextra -fsanitize=address,undefined -fno-sanitize-recover=all' # Gather the list of available components. These are the functions # defined in this script whose name starts with "component_". From 8b5389f360bfa95c613ba0657ef9b0b5c4a7e20b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 21 Oct 2019 19:08:07 +0200 Subject: [PATCH 3/4] 'make test' must fail if Asan fails When running 'make test' with GNU make, if a test suite program displays "PASSED", this was automatically counted as a pass. This would in particular count as passing: * A test suite with the substring "PASSED" in a test description. * A test suite where all the test cases succeeded, but the final cleanup failed, in particular if a sanitizer reported a memory leak. Use the test executable's return status instead to determine whether the test suite passed. It's always 0 on PASSED unless the executable's cleanup code fails, and it's never 0 on any failure. Fix ARMmbed/mbed-crypto#303 --- tests/scripts/run-test-suites.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/run-test-suites.pl b/tests/scripts/run-test-suites.pl index 1c9dc1dfc..d06badd23 100755 --- a/tests/scripts/run-test-suites.pl +++ b/tests/scripts/run-test-suites.pl @@ -93,7 +93,7 @@ for my $suite (@suites) $suite_cases_failed = () = $result =~ /.. FAILED/g; $suite_cases_skipped = () = $result =~ /.. ----/g; - if( $result =~ /PASSED/ ) { + if( $? == 0 ) { print "PASS\n"; if( $verbose > 2 ) { pad_print_center( 72, '-', "Begin $suite" ); From 54d193743395578f0ac53dd818d4e5eab26a9cc7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 21 Oct 2019 15:57:51 +0200 Subject: [PATCH 4/4] Fix memory leak in some SE HAL tests --- tests/suites/test_suite_psa_crypto_se_driver_hal.function | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index 61fb91805..e06ef1791 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -565,16 +565,17 @@ static int check_persistent_data( psa_key_lifetime_t lifetime, psa_storage_uid_t uid = file_uid_for_lifetime( lifetime ); struct psa_storage_info_t info; uint8_t *loaded = NULL; + int ok = 0; PSA_ASSERT( psa_its_get_info( uid, &info ) ); ASSERT_ALLOC( loaded, info.size ); PSA_ASSERT( psa_its_get( uid, 0, info.size, loaded, NULL ) ); ASSERT_COMPARE( expected_data, size, loaded, info.size ); - return( 1 ); + ok = 1; exit: mbedtls_free( loaded ); - return( 0 ); + return( ok ); } /* Check that a function's return status is "smoke-free", i.e. that