From c33940da511e1c26c43cd7945d00f6632a54d607 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 18:48:39 +0100 Subject: [PATCH 01/21] Add a metatest program This program can be used to validate that things that should be detected as test failures are indeed caught, either by setting the test result to MBEDTLS_TEST_RESULT_FAILED or by aborting the program. Signed-off-by: Gilles Peskine --- programs/.gitignore | 1 + programs/Makefile | 5 + programs/test/CMakeLists.txt | 1 + programs/test/metatest.c | 81 ++++++++++++++++ visualc/VS2010/mbedTLS.sln | 13 +++ visualc/VS2010/metatest.vcxproj | 167 ++++++++++++++++++++++++++++++++ 6 files changed, 268 insertions(+) create mode 100644 programs/test/metatest.c create mode 100644 visualc/VS2010/metatest.vcxproj diff --git a/programs/.gitignore b/programs/.gitignore index 59634b54e..3775216d5 100644 --- a/programs/.gitignore +++ b/programs/.gitignore @@ -53,6 +53,7 @@ test/cpp_dummy_build test/cpp_dummy_build.cpp test/dlopen test/ecp-bench +test/metatest test/query_compile_time_config test/selftest test/ssl_cert_test diff --git a/programs/Makefile b/programs/Makefile index 2d0f70582..43ae99f26 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -114,6 +114,7 @@ APPS = \ ssl/ssl_server$(EXEXT) \ ssl/ssl_server2$(EXEXT) \ test/benchmark$(EXEXT) \ + test/metatest$(EXEXT) \ test/query_compile_time_config$(EXEXT) \ test/selftest$(EXEXT) \ test/udp_proxy$(EXEXT) \ @@ -349,6 +350,10 @@ test/dlopen$(EXEXT): test/dlopen.c $(DEP) $(CC) $(LOCAL_CFLAGS) $(CFLAGS) test/dlopen.c $(LDFLAGS) $(DLOPEN_LDFLAGS) -o $@ endif +test/metatest$(EXEXT): test/metatest.c $(DEP) + echo " CC test/metatest.c" + $(CC) $(LOCAL_CFLAGS) $(CFLAGS) test/metatest.c $(LOCAL_LDFLAGS) $(LDFLAGS) -o $@ + test/query_config.o: test/query_config.c test/query_config.h $(DEP) echo " CC test/query_config.c" $(CC) $(LOCAL_CFLAGS) $(CFLAGS) -c test/query_config.c -o $@ diff --git a/programs/test/CMakeLists.txt b/programs/test/CMakeLists.txt index 403797cef..98757f5d8 100644 --- a/programs/test/CMakeLists.txt +++ b/programs/test/CMakeLists.txt @@ -11,6 +11,7 @@ if(ENABLE_ZLIB_SUPPORT) endif(ENABLE_ZLIB_SUPPORT) set(executables_libs + metatest selftest udp_proxy ) diff --git a/programs/test/metatest.c b/programs/test/metatest.c new file mode 100644 index 000000000..f03e5e703 --- /dev/null +++ b/programs/test/metatest.c @@ -0,0 +1,81 @@ +/** \file metatest.c + * + * \brief Test features of the test framework. + */ + +/* + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later + */ + +#define MBEDTLS_ALLOW_PRIVATE_ACCESS + +#include +#include "test/helpers.h" + +#include +#include + + +/****************************************************************/ +/* Command line entry point */ +/****************************************************************/ + +typedef struct { + const char *name; + const char *platform; + void (*entry_point)(const char *name); +} metatest_t; + +metatest_t metatests[] = { + { NULL, NULL, NULL } +}; + +static void help(FILE *out, const char *argv0) +{ + mbedtls_fprintf(out, "Usage: %s list|TEST\n", argv0); + mbedtls_fprintf(out, "Run a meta-test that should cause a test failure.\n"); + mbedtls_fprintf(out, "With 'list', list the available tests and their platform requirement.\n"); +} + +int main(int argc, char *argv[]) +{ + const char *argv0 = argc > 0 ? argv[0] : "metatest"; + if (argc != 2) { + help(stderr, argv0); + mbedtls_exit(MBEDTLS_EXIT_FAILURE); + } + + /* Support "-help", "--help", "--list", etc. */ + const char *command = argv[1]; + while (*command == '-') { + ++command; + } + + if (strcmp(argv[1], "help") == 0) { + help(stdout, argv0); + mbedtls_exit(MBEDTLS_EXIT_SUCCESS); + } + if (strcmp(argv[1], "list") == 0) { + for (const metatest_t *p = metatests; p->name != NULL; p++) { + mbedtls_printf("%s %s\n", p->name, p->platform); + } + mbedtls_exit(MBEDTLS_EXIT_SUCCESS); + } + + for (const metatest_t *p = metatests; p->name != NULL; p++) { + if (strcmp(argv[1], p->name) == 0) { + mbedtls_printf("Running metatest %s...\n", argv[1]); + p->entry_point(argv[1]); + mbedtls_printf("Running metatest %s... done, result=%d\n", + argv[1], (int) mbedtls_test_info.result); + mbedtls_exit(mbedtls_test_info.result == MBEDTLS_TEST_RESULT_SUCCESS ? + MBEDTLS_EXIT_SUCCESS : + MBEDTLS_EXIT_FAILURE); + } + } + + mbedtls_fprintf(stderr, "%s: FATAL: No such metatest: %s\n", + argv0, command); + mbedtls_exit(MBEDTLS_EXIT_FAILURE); +} diff --git a/visualc/VS2010/mbedTLS.sln b/visualc/VS2010/mbedTLS.sln index 362473511..995ba52a2 100644 --- a/visualc/VS2010/mbedTLS.sln +++ b/visualc/VS2010/mbedTLS.sln @@ -203,6 +203,11 @@ Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "benchmark", "benchmark.vcxp {46CF2D25-6A36-4189-B59C-E4815388E554} = {46CF2D25-6A36-4189-B59C-E4815388E554} EndProjectSection EndProject +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "metatest", "metatest.vcxproj", "{95B15C5B-0EB4-4353-7990-22F6965A9437}" + ProjectSection(ProjectDependencies) = postProject + {46CF2D25-6A36-4189-B59C-E4815388E554} = {46CF2D25-6A36-4189-B59C-E4815388E554} + EndProjectSection +EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "query_compile_time_config", "query_compile_time_config.vcxproj", "{D6F58AF2-9D80-562A-E2B0-F743281522B9}" ProjectSection(ProjectDependencies) = postProject {46CF2D25-6A36-4189-B59C-E4815388E554} = {46CF2D25-6A36-4189-B59C-E4815388E554} @@ -599,6 +604,14 @@ Global {90EFD9A4-C6B0-3EE8-1F06-0A0E0D55AEDA}.Release|Win32.Build.0 = Release|Win32 {90EFD9A4-C6B0-3EE8-1F06-0A0E0D55AEDA}.Release|x64.ActiveCfg = Release|x64 {90EFD9A4-C6B0-3EE8-1F06-0A0E0D55AEDA}.Release|x64.Build.0 = Release|x64 + {95B15C5B-0EB4-4353-7990-22F6965A9437}.Debug|Win32.ActiveCfg = Debug|Win32 + {95B15C5B-0EB4-4353-7990-22F6965A9437}.Debug|Win32.Build.0 = Debug|Win32 + {95B15C5B-0EB4-4353-7990-22F6965A9437}.Debug|x64.ActiveCfg = Debug|x64 + {95B15C5B-0EB4-4353-7990-22F6965A9437}.Debug|x64.Build.0 = Debug|x64 + {95B15C5B-0EB4-4353-7990-22F6965A9437}.Release|Win32.ActiveCfg = Release|Win32 + {95B15C5B-0EB4-4353-7990-22F6965A9437}.Release|Win32.Build.0 = Release|Win32 + {95B15C5B-0EB4-4353-7990-22F6965A9437}.Release|x64.ActiveCfg = Release|x64 + {95B15C5B-0EB4-4353-7990-22F6965A9437}.Release|x64.Build.0 = Release|x64 {D6F58AF2-9D80-562A-E2B0-F743281522B9}.Debug|Win32.ActiveCfg = Debug|Win32 {D6F58AF2-9D80-562A-E2B0-F743281522B9}.Debug|Win32.Build.0 = Debug|Win32 {D6F58AF2-9D80-562A-E2B0-F743281522B9}.Debug|x64.ActiveCfg = Debug|x64 diff --git a/visualc/VS2010/metatest.vcxproj b/visualc/VS2010/metatest.vcxproj new file mode 100644 index 000000000..0d9bb8a3f --- /dev/null +++ b/visualc/VS2010/metatest.vcxproj @@ -0,0 +1,167 @@ + + + + + Debug + Win32 + + + Debug + x64 + + + Release + Win32 + + + Release + x64 + + + + + + + + {46cf2d25-6a36-4189-b59c-e4815388e554} + true + + + + {95B15C5B-0EB4-4353-7990-22F6965A9437} + Win32Proj + metatest + + + + Application + true + Unicode + + + Application + true + Unicode + + + Application + false + true + Unicode + + + Application + false + true + Unicode + + + + + + + + + + + + + + + + + + + true + $(Configuration)\$(TargetName)\ + + + true + $(Configuration)\$(TargetName)\ + + + false + $(Configuration)\$(TargetName)\ + + + false + $(Configuration)\$(TargetName)\ + + + + Level3 + Disabled + %(PreprocessorDefinitions) + +../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include + + + Console + true + kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + Debug + + + false + + + + + Level3 + Disabled + %(PreprocessorDefinitions) + +../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include + + + Console + true + kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + Debug + + + false + + + + + Level3 + MaxSpeed + true + true + NDEBUG;%(PreprocessorDefinitions) + +../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include + + + Console + true + true + true + Release + kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + + + + + Level3 + MaxSpeed + true + true + NDEBUG;%(PreprocessorDefinitions) + +../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include + + + Console + true + true + true + Release + %(AdditionalDependencies); + + + + + + From 30380dacdb7c62e6bae1a0ecab156cf81b6e25a0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 18:49:52 +0100 Subject: [PATCH 02/21] Validate that test_fail causes a test failure Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index f03e5e703..fab3b1f53 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -17,6 +17,17 @@ #include +/****************************************************************/ +/* Test framework features */ +/****************************************************************/ + +void meta_test_fail(const char *name) +{ + (void) name; + mbedtls_test_fail("Forced test failure", __LINE__, __FILE__); +} + + /****************************************************************/ /* Command line entry point */ /****************************************************************/ @@ -28,6 +39,7 @@ typedef struct { } metatest_t; metatest_t metatests[] = { + { "test_fail", "any", meta_test_fail }, { NULL, NULL, NULL } }; From 21d8d59ce2a137b9f38ae75bf5e9f0878fa43e7d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 19:23:26 +0100 Subject: [PATCH 03/21] Metatests for null pointer dereference Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index fab3b1f53..ba3ec9443 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -28,6 +28,29 @@ void meta_test_fail(const char *name) } +/****************************************************************/ +/* Platform features */ +/****************************************************************/ + +void null_pointer_dereference(const char *name) +{ + (void) name; + char *p; + memset(&p, 0, sizeof(p)); + volatile char c; + c = *p; + (void) c; +} + +void null_pointer_call(const char *name) +{ + (void) name; + void (*p)(void); + memset(&p, 0, sizeof(p)); + p(); +} + + /****************************************************************/ /* Command line entry point */ /****************************************************************/ @@ -40,6 +63,8 @@ typedef struct { metatest_t metatests[] = { { "test_fail", "any", meta_test_fail }, + { "null_dereference", "any", null_pointer_dereference }, + { "null_call", "any", null_pointer_call }, { NULL, NULL, NULL } }; From 6effdff76bc2b3f65222902d0aaee391b169f00a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 19:23:41 +0100 Subject: [PATCH 04/21] Script to run all the metatests (with platform filtering) Signed-off-by: Gilles Peskine --- tests/scripts/run-metatests.sh | 81 ++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100755 tests/scripts/run-metatests.sh diff --git a/tests/scripts/run-metatests.sh b/tests/scripts/run-metatests.sh new file mode 100755 index 000000000..182bf0410 --- /dev/null +++ b/tests/scripts/run-metatests.sh @@ -0,0 +1,81 @@ +#!/bin/sh + +help () { + cat <&2 "$0: FATAL: programs/test/metatest not found" + exit 120 +fi + +LIST_ONLY= +while getopts hl OPTLET; do + case $OPTLET in + h) help; exit;; + l) LIST_ONLY=1;; + \?) help >&2; exit 120;; + esac +done +shift $((OPTIND - 1)) + +list_matches () { + while read name platform junk; do + for pattern; do + case $platform in + $pattern) echo "$name"; break;; + esac + done + done +} + +count=0 +errors=0 +run_metatest () { + ret=0 + "$METATEST_PROGRAM" "$1" || ret=$? + if [ $ret -eq 0 ]; then + echo >&2 "$0: Unexpected success: $1" + errors=$((errors + 1)) + fi + count=$((count + 1)) +} + +# Don't pipe the output of metatest so that if it fails, this script exits +# immediately with a failure status. +full_list=$("$METATEST_PROGRAM" list) +matching_list=$(printf '%s\n' "$full_list" | list_matches "$@") + +if [ -n "$LIST_ONLY" ]; then + printf '%s\n' $matching_list + exit +fi + +for name in $matching_list; do + run_metatest "$name" +done + +if [ $errors -eq 0 ]; then + echo "Ran $count metatests, all good." + exit 0 +else + echo "Ran $count metatests, $errors unexpected successes." + exit 1 +fi From 970584f32fef06f3b2c88c439ba3665f646fdd1e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 19:42:13 +0100 Subject: [PATCH 05/21] Metatests for basic Asan and Msan features Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 48 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index ba3ec9443..91a1e2a7d 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -51,6 +51,50 @@ void null_pointer_call(const char *name) } +/****************************************************************/ +/* Sanitizers */ +/****************************************************************/ + +void read_after_free(const char *name) +{ + (void) name; + volatile char *p = mbedtls_calloc(1, 1); + *p = 'a'; + mbedtls_free((void *) p); + mbedtls_printf("%u\n", (unsigned) *p); +} + +void double_free(const char *name) +{ + (void) name; + volatile char *p = mbedtls_calloc(1, 1); + *p = 'a'; + mbedtls_free((void *) p); + mbedtls_free((void *) p); +} + +void read_uninitialized_stack(const char *name) +{ + (void) name; + volatile char buf[1]; + static int false_but_the_compiler_does_not_know = 0; + if (false_but_the_compiler_does_not_know) { + buf[0] = '!'; + } + if (*buf != 0) { + mbedtls_printf("%u\n", (unsigned) *buf); + } +} + +void memory_leak(const char *name) +{ + (void) name; + volatile char *p = mbedtls_calloc(1, 1); + /* Hint to the compiler that calloc must not be optimized away. */ + (void) *p; +} + + /****************************************************************/ /* Command line entry point */ /****************************************************************/ @@ -65,6 +109,10 @@ metatest_t metatests[] = { { "test_fail", "any", meta_test_fail }, { "null_dereference", "any", null_pointer_dereference }, { "null_call", "any", null_pointer_call }, + { "read_after_free", "asan", read_after_free }, + { "double_free", "asan", double_free }, + { "read_uninitialized_stack", "msan", read_uninitialized_stack }, + { "memory_leak", "asan", memory_leak }, { NULL, NULL, NULL } }; From 967714d8e77c00c08f061243a6fc2aa526e93c36 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 19:52:32 +0100 Subject: [PATCH 06/21] Strengthen against Clang optimizations Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index 91a1e2a7d..eb3bb76af 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -11,12 +11,21 @@ #define MBEDTLS_ALLOW_PRIVATE_ACCESS #include +#include #include "test/helpers.h" #include #include +/* This is an external variable, so the compiler doesn't know that we're never + * changing its value. + * + * TODO: LTO (link-time-optimization) would defeat this. + */ +int false_but_the_compiler_does_not_know = 0; + + /****************************************************************/ /* Test framework features */ /****************************************************************/ @@ -35,19 +44,17 @@ void meta_test_fail(const char *name) void null_pointer_dereference(const char *name) { (void) name; - char *p; - memset(&p, 0, sizeof(p)); - volatile char c; - c = *p; - (void) c; + volatile char *p; + mbedtls_platform_zeroize(&p, sizeof(p)); + mbedtls_printf("%p -> %u\n", p, (unsigned) *p); } void null_pointer_call(const char *name) { (void) name; - void (*p)(void); - memset(&p, 0, sizeof(p)); - p(); + unsigned (*p)(void); + mbedtls_platform_zeroize(&p, sizeof(p)); + mbedtls_printf("%p() -> %u\n", p, p()); } @@ -77,7 +84,6 @@ void read_uninitialized_stack(const char *name) { (void) name; volatile char buf[1]; - static int false_but_the_compiler_does_not_know = 0; if (false_but_the_compiler_does_not_know) { buf[0] = '!'; } From ee8af06887a397189156bf85aee52b324e609fc4 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 19:58:03 +0100 Subject: [PATCH 07/21] Run metatests in selected components Run metatests in some components, covering both GCC and Clang, with ASan, MSan or neither. Note that this commit does not cover constant-flow testing builds or Valgrind. Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 69f141f92..23ef57840 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -875,6 +875,9 @@ component_test_default_cmake_gcc_asan () { msg "test: selftest (ASan build)" # ~ 10s programs/test/selftest + msg "test: metatests (GCC, ASan build)" + tests/scripts/run-metatests.sh any asan + msg "test: ssl-opt.sh (ASan build)" # ~ 1 min tests/ssl-opt.sh @@ -1482,6 +1485,9 @@ component_test_everest () { msg "test: Everest ECDH context - main suites (inc. selftests) (ASan build)" # ~ 50s make test + msg "test: metatests (clang, ASan)" + tests/scripts/run-metatests.sh any asan + msg "test: Everest ECDH context - ECDH-related part of ssl-opt.sh (ASan build)" # ~ 5s tests/ssl-opt.sh -f ECDH @@ -1572,6 +1578,9 @@ component_test_full_cmake_clang () { msg "test: cpp_dummy_build (full config, clang)" # ~ 1s programs/test/cpp_dummy_build + msg "test: metatests (clang)" + tests/scripts/run-metatests.sh any + msg "program demos (full config, clang)" # ~10s tests/scripts/run_demos.py @@ -3345,6 +3354,9 @@ component_test_memsan () { msg "test: main suites (MSan)" # ~ 10s make test + msg "test: metatests (MSan)" + tests/scripts/run-metatests.sh any msan + msg "program demos (MSan)" # ~20s tests/scripts/run_demos.py From 6289ccc006c847b398744ca5bec9a79f258f64b9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 21:31:07 +0100 Subject: [PATCH 08/21] Use casts when doing nonstandard pointer conversions Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index eb3bb76af..36119f68d 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -45,7 +45,7 @@ void null_pointer_dereference(const char *name) { (void) name; volatile char *p; - mbedtls_platform_zeroize(&p, sizeof(p)); + mbedtls_platform_zeroize((void *) &p, sizeof(p)); mbedtls_printf("%p -> %u\n", p, (unsigned) *p); } @@ -54,7 +54,7 @@ void null_pointer_call(const char *name) (void) name; unsigned (*p)(void); mbedtls_platform_zeroize(&p, sizeof(p)); - mbedtls_printf("%p() -> %u\n", p, p()); + mbedtls_printf("%p() -> %u\n", (void *) p, p()); } From 64c6c80e28a7e7084b28ede34388c4788c79afc9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 22:32:50 +0100 Subject: [PATCH 09/21] Don't cast a function pointer to a data pointer That's nonstandard. Instead, convert to an integer. Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index 36119f68d..872dbf0f8 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -54,7 +54,7 @@ void null_pointer_call(const char *name) (void) name; unsigned (*p)(void); mbedtls_platform_zeroize(&p, sizeof(p)); - mbedtls_printf("%p() -> %u\n", (void *) p, p()); + mbedtls_printf("%llx() -> %u\n", (unsigned long long) p, p()); } From 6b3b7f832b42de9fdcffd364aa43fe0985a39fdf Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 3 Nov 2023 10:31:56 +0100 Subject: [PATCH 10/21] Fix cast from pointer to integer of different size Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index 872dbf0f8..1a638b595 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -54,7 +54,7 @@ void null_pointer_call(const char *name) (void) name; unsigned (*p)(void); mbedtls_platform_zeroize(&p, sizeof(p)); - mbedtls_printf("%llx() -> %u\n", (unsigned long long) p, p()); + mbedtls_printf("%llx() -> %u\n", (unsigned long long) (uintptr_t) p, p()); } From db2b5c94a65c6c178ddb723c9740bdcdab5db22d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 3 Nov 2023 10:58:57 +0100 Subject: [PATCH 11/21] Don't use %llx in printf We still do MinGW builds on our CI whose printf doesn't support it! Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index 1a638b595..d3173f3d4 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -54,7 +54,10 @@ void null_pointer_call(const char *name) (void) name; unsigned (*p)(void); mbedtls_platform_zeroize(&p, sizeof(p)); - mbedtls_printf("%llx() -> %u\n", (unsigned long long) (uintptr_t) p, p()); + /* The pointer representation may be truncated, but we don't care: + * the only point of printing it is to have some use of the pointer + * to dissuade the compiler from optimizing it away. */ + mbedtls_printf("%lx() -> %u\n", (unsigned long) (uintptr_t) p, p()); } From e38eb79e890d86138b74712502b40a68dd4681ae Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 3 Nov 2023 18:05:38 +0100 Subject: [PATCH 12/21] Add metatests for mutex usage Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 100 ++++++++++++++++++++++++++++++++++++++- tests/scripts/all.sh | 2 +- 2 files changed, 100 insertions(+), 2 deletions(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index d3173f3d4..7e173ee27 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -13,10 +13,15 @@ #include #include #include "test/helpers.h" +#include "test/macros.h" #include #include +#if defined(MBEDTLS_THREADING_C) +#include +#endif + /* This is an external variable, so the compiler doesn't know that we're never * changing its value. @@ -62,7 +67,7 @@ void null_pointer_call(const char *name) /****************************************************************/ -/* Sanitizers */ +/* Memory */ /****************************************************************/ void read_after_free(const char *name) @@ -104,6 +109,84 @@ void memory_leak(const char *name) } +/****************************************************************/ +/* Threading */ +/****************************************************************/ + +void mutex_lock_not_initialized(const char *name) +{ + (void) name; + /* Mutex usage verification is only done with pthread, not with other + * threading implementations. See tests/src/threading_helpers.c. */ +#if defined(MBEDTLS_THREADING_PTHREAD) + mbedtls_threading_mutex_t mutex; + memset(&mutex, 0, sizeof(mutex)); + TEST_ASSERT(mbedtls_mutex_lock(&mutex) == 0); +exit: + ; +#endif +} + +void mutex_unlock_not_initialized(const char *name) +{ + (void) name; + /* Mutex usage verification is only done with pthread, not with other + * threading implementations. See tests/src/threading_helpers.c. */ +#if defined(MBEDTLS_THREADING_C) + mbedtls_threading_mutex_t mutex; + memset(&mutex, 0, sizeof(mutex)); + TEST_ASSERT(mbedtls_mutex_unlock(&mutex) == 0); +exit: + ; +#endif +} + +void mutex_free_not_initialized(const char *name) +{ + (void) name; + /* Mutex usage verification is only done with pthread, not with other + * threading implementations. See tests/src/threading_helpers.c. */ +#if defined(MBEDTLS_THREADING_C) + mbedtls_threading_mutex_t mutex; + memset(&mutex, 0, sizeof(mutex)); + mbedtls_mutex_free(&mutex); +#endif +} + +void mutex_double_init(const char *name) +{ + (void) name; +#if defined(MBEDTLS_THREADING_C) + mbedtls_threading_mutex_t mutex; + mbedtls_mutex_init(&mutex); + mbedtls_mutex_init(&mutex); + mbedtls_mutex_free(&mutex); +#endif +} + +void mutex_double_free(const char *name) +{ + (void) name; +#if defined(MBEDTLS_THREADING_C) + mbedtls_threading_mutex_t mutex; + mbedtls_mutex_init(&mutex); + mbedtls_mutex_free(&mutex); + mbedtls_mutex_free(&mutex); +#endif +} + +void mutex_leak(const char *name) +{ + (void) name; + /* Mutex usage verification is only done with pthread, not with other + * threading implementations. See tests/src/threading_helpers.c. */ +#if defined(MBEDTLS_THREADING_PTHREAD) + mbedtls_threading_mutex_t mutex; + mbedtls_mutex_init(&mutex); +#endif +} + + /****************************************************************/ /* Command line entry point */ /****************************************************************/ @@ -122,6 +205,14 @@ metatest_t metatests[] = { { "double_free", "asan", double_free }, { "read_uninitialized_stack", "msan", read_uninitialized_stack }, { "memory_leak", "asan", memory_leak }, + /* Mutex usage verification is only done with pthread, not with other + * threading implementations. See tests/src/threading_helpers.c. */ + { "mutex_lock_not_initialized", "pthread", mutex_lock_not_initialized }, + { "mutex_unlock_not_initialized", "pthread", mutex_unlock_not_initialized }, + { "mutex_free_not_initialized", "pthread", mutex_free_not_initialized }, + { "mutex_double_init", "pthread", mutex_double_init }, + { "mutex_double_free", "pthread", mutex_double_free }, + { "mutex_leak", "pthread", mutex_leak }, { NULL, NULL, NULL } }; @@ -157,10 +248,17 @@ int main(int argc, char *argv[]) mbedtls_exit(MBEDTLS_EXIT_SUCCESS); } +#if defined(MBEDTLS_TEST_MUTEX_USAGE) + mbedtls_test_mutex_usage_init(); +#endif + for (const metatest_t *p = metatests; p->name != NULL; p++) { if (strcmp(argv[1], p->name) == 0) { mbedtls_printf("Running metatest %s...\n", argv[1]); p->entry_point(argv[1]); +#if defined(MBEDTLS_TEST_MUTEX_USAGE) + mbedtls_test_mutex_usage_check(); +#endif mbedtls_printf("Running metatest %s... done, result=%d\n", argv[1], (int) mbedtls_test_info.result); mbedtls_exit(mbedtls_test_info.result == MBEDTLS_TEST_RESULT_SUCCESS ? diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 23ef57840..aae020976 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1579,7 +1579,7 @@ component_test_full_cmake_clang () { programs/test/cpp_dummy_build msg "test: metatests (clang)" - tests/scripts/run-metatests.sh any + tests/scripts/run-metatests.sh any pthread msg "program demos (full config, clang)" # ~10s tests/scripts/run_demos.py From 510c01c6ad5c0a88b68e79cbfe5a1408735011ab Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 3 Nov 2023 18:57:06 +0100 Subject: [PATCH 13/21] Add missing program to .gitignore Signed-off-by: Gilles Peskine --- programs/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/programs/.gitignore b/programs/.gitignore index 3775216d5..8420c815c 100644 --- a/programs/.gitignore +++ b/programs/.gitignore @@ -34,6 +34,7 @@ pkey/rsa_verify_pss psa/crypto_examples psa/key_ladder_demo psa/psa_constant_names +psa/psa_hash random/gen_entropy random/gen_random_ctr_drbg random/gen_random_havege From 2c04f57ffc83dec9df1cd002d5af6b883691665b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 3 Nov 2023 19:16:56 +0100 Subject: [PATCH 14/21] programs/test/metatest indirectly includes library/common.h Signed-off-by: Gilles Peskine --- programs/Makefile | 2 +- programs/test/CMakeLists.txt | 2 ++ scripts/generate_visualc_files.pl | 5 ++++- visualc/VS2010/benchmark.vcxproj | 8 ++++---- visualc/VS2010/metatest.vcxproj | 8 ++++---- visualc/VS2010/query_compile_time_config.vcxproj | 8 ++++---- visualc/VS2010/selftest.vcxproj | 8 ++++---- visualc/VS2010/udp_proxy.vcxproj | 8 ++++---- visualc/VS2010/zeroize.vcxproj | 8 ++++---- 9 files changed, 31 insertions(+), 26 deletions(-) diff --git a/programs/Makefile b/programs/Makefile index 43ae99f26..2d32e0028 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -352,7 +352,7 @@ endif test/metatest$(EXEXT): test/metatest.c $(DEP) echo " CC test/metatest.c" - $(CC) $(LOCAL_CFLAGS) $(CFLAGS) test/metatest.c $(LOCAL_LDFLAGS) $(LDFLAGS) -o $@ + $(CC) $(LOCAL_CFLAGS) $(CFLAGS) -I ../library test/metatest.c $(LOCAL_LDFLAGS) $(LDFLAGS) -o $@ test/query_config.o: test/query_config.c test/query_config.h $(DEP) echo " CC test/query_config.c" diff --git a/programs/test/CMakeLists.txt b/programs/test/CMakeLists.txt index 98757f5d8..662c5ff6a 100644 --- a/programs/test/CMakeLists.txt +++ b/programs/test/CMakeLists.txt @@ -51,6 +51,8 @@ foreach(exe IN LISTS executables_libs executables_mbedcrypto) endif() add_executable(${exe} ${exe}.c $ ${extra_sources}) + target_include_directories(${exe} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../../tests/include) + target_include_directories(${exe} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../../library) # This emulates "if ( ... IN_LIST ... )" which becomes available in CMake 3.3 list(FIND executables_libs ${exe} exe_index) diff --git a/scripts/generate_visualc_files.pl b/scripts/generate_visualc_files.pl index ab2c93d19..11604cd9c 100755 --- a/scripts/generate_visualc_files.pl +++ b/scripts/generate_visualc_files.pl @@ -145,6 +145,7 @@ sub gen_app { my $guid = gen_app_guid( $path ); $path =~ s!/!\\!g; (my $appname = $path) =~ s/.*\\//; + my $is_test_app = ($path =~ m/^test\\/); my $srcs = ""; if( $appname eq "ssl_client2" or $appname eq "ssl_server2" or @@ -159,7 +160,9 @@ sub gen_app { $content =~ s//$srcs/g; $content =~ s//$appname/g; $content =~ s//$guid/g; - $content =~ s/INCLUDE_DIRECTORIES\r\n/$include_directories/g; + $content =~ s/INCLUDE_DIRECTORIES\r\n/($is_test_app ? + $library_include_directories : + $include_directories)/ge; content_to_file( $content, "$dir/$appname.$ext" ); } diff --git a/visualc/VS2010/benchmark.vcxproj b/visualc/VS2010/benchmark.vcxproj index 0be32fc5a..ab27b6c50 100644 --- a/visualc/VS2010/benchmark.vcxproj +++ b/visualc/VS2010/benchmark.vcxproj @@ -93,7 +93,7 @@ Disabled %(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -111,7 +111,7 @@ Disabled %(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -131,7 +131,7 @@ true NDEBUG;%(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -150,7 +150,7 @@ true NDEBUG;%(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console diff --git a/visualc/VS2010/metatest.vcxproj b/visualc/VS2010/metatest.vcxproj index 0d9bb8a3f..b1addb6c9 100644 --- a/visualc/VS2010/metatest.vcxproj +++ b/visualc/VS2010/metatest.vcxproj @@ -93,7 +93,7 @@ Disabled %(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -111,7 +111,7 @@ Disabled %(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -131,7 +131,7 @@ true NDEBUG;%(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -150,7 +150,7 @@ true NDEBUG;%(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console diff --git a/visualc/VS2010/query_compile_time_config.vcxproj b/visualc/VS2010/query_compile_time_config.vcxproj index d0e0a6df6..97bfad435 100644 --- a/visualc/VS2010/query_compile_time_config.vcxproj +++ b/visualc/VS2010/query_compile_time_config.vcxproj @@ -94,7 +94,7 @@ Disabled %(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -112,7 +112,7 @@ Disabled %(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -132,7 +132,7 @@ true NDEBUG;%(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -151,7 +151,7 @@ true NDEBUG;%(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console diff --git a/visualc/VS2010/selftest.vcxproj b/visualc/VS2010/selftest.vcxproj index 6feb5936f..36299018a 100644 --- a/visualc/VS2010/selftest.vcxproj +++ b/visualc/VS2010/selftest.vcxproj @@ -93,7 +93,7 @@ Disabled %(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -111,7 +111,7 @@ Disabled %(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -131,7 +131,7 @@ true NDEBUG;%(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -150,7 +150,7 @@ true NDEBUG;%(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console diff --git a/visualc/VS2010/udp_proxy.vcxproj b/visualc/VS2010/udp_proxy.vcxproj index 69678f635..993fbc04e 100644 --- a/visualc/VS2010/udp_proxy.vcxproj +++ b/visualc/VS2010/udp_proxy.vcxproj @@ -93,7 +93,7 @@ Disabled %(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -111,7 +111,7 @@ Disabled %(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -131,7 +131,7 @@ true NDEBUG;%(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -150,7 +150,7 @@ true NDEBUG;%(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console diff --git a/visualc/VS2010/zeroize.vcxproj b/visualc/VS2010/zeroize.vcxproj index 9e0746de9..f70896f7b 100644 --- a/visualc/VS2010/zeroize.vcxproj +++ b/visualc/VS2010/zeroize.vcxproj @@ -93,7 +93,7 @@ Disabled %(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -111,7 +111,7 @@ Disabled %(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -131,7 +131,7 @@ true NDEBUG;%(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console @@ -150,7 +150,7 @@ true NDEBUG;%(PreprocessorDefinitions) -../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include +../../library;../../include;../../3rdparty/everest/include/;../../3rdparty/everest/include/everest;../../3rdparty/everest/include/everest/vs2010;../../3rdparty/everest/include/everest/kremlib;../../tests/include Console From 53833516bfd35c0bee2967546f13bb6ae97f8553 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 9 Nov 2023 21:46:24 +0100 Subject: [PATCH 15/21] Strengthen against possible compiler optimizations Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index 7e173ee27..805de2d30 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -25,10 +25,15 @@ /* This is an external variable, so the compiler doesn't know that we're never * changing its value. - * - * TODO: LTO (link-time-optimization) would defeat this. */ -int false_but_the_compiler_does_not_know = 0; +volatile int false_but_the_compiler_does_not_know = 0; + +/* Set n bytes at the address p to all-bits-zero, in such a way that + * the compiler should not know that p is all-bits-zero. */ +static void set_to_zero_but_the_compiler_does_not_know(void *p, size_t n) +{ + memset(p, false_but_the_compiler_does_not_know, n); +} /****************************************************************/ @@ -50,7 +55,7 @@ void null_pointer_dereference(const char *name) { (void) name; volatile char *p; - mbedtls_platform_zeroize((void *) &p, sizeof(p)); + set_to_zero_but_the_compiler_does_not_know(&p, sizeof(p)); mbedtls_printf("%p -> %u\n", p, (unsigned) *p); } @@ -58,7 +63,7 @@ void null_pointer_call(const char *name) { (void) name; unsigned (*p)(void); - mbedtls_platform_zeroize(&p, sizeof(p)); + set_to_zero_but_the_compiler_does_not_know(&p, sizeof(p)); /* The pointer representation may be truncated, but we don't care: * the only point of printing it is to have some use of the pointer * to dissuade the compiler from optimizing it away. */ @@ -104,8 +109,7 @@ void memory_leak(const char *name) { (void) name; volatile char *p = mbedtls_calloc(1, 1); - /* Hint to the compiler that calloc must not be optimized away. */ - (void) *p; + mbedtls_printf("%u\n", (unsigned) *p); } From 226f1bc08090878f37b4d6abcbd3d228a60619e7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 10 Nov 2023 10:09:27 +0100 Subject: [PATCH 16/21] More consistent usage of volatile Fix MSVC warning C4090. Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index 805de2d30..ce866edec 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -30,9 +30,9 @@ volatile int false_but_the_compiler_does_not_know = 0; /* Set n bytes at the address p to all-bits-zero, in such a way that * the compiler should not know that p is all-bits-zero. */ -static void set_to_zero_but_the_compiler_does_not_know(void *p, size_t n) +static void set_to_zero_but_the_compiler_does_not_know(volatile void *p, size_t n) { - memset(p, false_but_the_compiler_does_not_know, n); + memset((void *) p, false_but_the_compiler_does_not_know, n); } @@ -54,7 +54,7 @@ void meta_test_fail(const char *name) void null_pointer_dereference(const char *name) { (void) name; - volatile char *p; + volatile char *volatile p; set_to_zero_but_the_compiler_does_not_know(&p, sizeof(p)); mbedtls_printf("%p -> %u\n", p, (unsigned) *p); } @@ -62,7 +62,7 @@ void null_pointer_dereference(const char *name) void null_pointer_call(const char *name) { (void) name; - unsigned (*p)(void); + unsigned(*volatile p)(void); set_to_zero_but_the_compiler_does_not_know(&p, sizeof(p)); /* The pointer representation may be truncated, but we don't care: * the only point of printing it is to have some use of the pointer From efc57cabd0f2c3c56a9ddb80b8cdb933e1ffd856 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 10 Nov 2023 11:35:36 +0100 Subject: [PATCH 17/21] Uninitialized read: make the pointer non-volatile rather than the buffer Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index ce866edec..5e6a15f1d 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -96,12 +96,13 @@ void double_free(const char *name) void read_uninitialized_stack(const char *name) { (void) name; - volatile char buf[1]; + char buf[1]; if (false_but_the_compiler_does_not_know) { buf[0] = '!'; } - if (*buf != 0) { - mbedtls_printf("%u\n", (unsigned) *buf); + char *volatile p = buf; + if (*p != 0) { + mbedtls_printf("%u\n", (unsigned) *p); } } From c41133b90d97e172ebdfb9fc2bdc1a112cc9685a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 10 Nov 2023 15:36:15 +0100 Subject: [PATCH 18/21] Add documentation Explain the goals of metatests, how to write them, and how to read their output. Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 48 ++++++++++++++++++++++++++++++++++ tests/scripts/run-metatests.sh | 8 ++++++ 2 files changed, 56 insertions(+) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index 5e6a15f1d..c35e9a952 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -1,6 +1,24 @@ /** \file metatest.c * * \brief Test features of the test framework. + * + * When you run this program, it runs a single "meta-test". A meta-test + * performs an operation which should be caught as a failure by our + * test framework. The meta-test passes if this program calls `exit` with + * a nonzero status, or aborts, or is terminated by a signal, or if the + * framework running the program considers the run an error (this happens + * with Valgrind for a memory leak). The non-success of the meta-test + * program means that the test failure has been caught correctly. + * + * Some failures are purely functional: the logic of the code causes the + * test result to be set to FAIL. Other failures come from extra + * instrumentation which is not present in a normal build; for example, + * Asan or Valgrind to detect memory leaks. This is reflected by the + * "platform" associated with each meta-test. + * + * Use the companion script `tests/scripts/run-metatests.sh` to run all + * the meta-tests for a given platform and validate that they trigger a + * detected failure as expected. */ /* @@ -197,11 +215,41 @@ void mutex_leak(const char *name) /****************************************************************/ typedef struct { + /** Command line argument that will trigger that metatest. + * + * Conventionally matches "[a-z0-9_]+". */ const char *name; + + /** Platform under which that metatest is valid. + * + * - "any": should work anywhere. + * - "asan": triggers ASan (Address Sanitizer). + * - "msan": triggers MSan (Memory Sanitizer). + * - "pthread": requires MBEDTLS_THREADING_PTHREAD and MBEDTLS_TEST_HOOKS. + */ const char *platform; + + /** Function that performs the metatest. + * + * The function receives the name as an argument. This allows using the + * same function to perform multiple variants of a test based on the name. + * + * When executed on a conforming platform, the function is expected to + * either cause a test failure (mbedtls_test_fail()), or cause the + * program to abort in some way (e.g. by causing a segfault or by + * triggering a sanitizer). + * + * When executed on a non-conforming platform, the function may return + * normally or may have unpredictable behavior. + */ void (*entry_point)(const char *name); } metatest_t; +/* The list of availble meta-tests. Remember to register new functions here! + * + * Note that we always compile all the functions, so that `metatest --list` + * will always list all the available meta-tests. + */ metatest_t metatests[] = { { "test_fail", "any", meta_test_fail }, { "null_dereference", "any", null_pointer_dereference }, diff --git a/tests/scripts/run-metatests.sh b/tests/scripts/run-metatests.sh index 182bf0410..09a6f8a4f 100755 --- a/tests/scripts/run-metatests.sh +++ b/tests/scripts/run-metatests.sh @@ -6,6 +6,14 @@ Usage: $0 [OPTION] [PLATFORM]... Run all the metatests whose platform matches any of the given PLATFORM. A PLATFORM can contain shell wildcards. +Expected output: a lot of scary-looking error messages, since each +metatest is expected to report a failure. The final line should be +"Ran N metatests, all good." + +If something goes wrong: the final line should be +"Ran N metatests, X unexpected successes". Look for "Unexpected success" +in the logs above. + -l List the available metatests, don't run them. EOF } From d4084fd89944afed5c47f931a1e805ed016ae960 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 15 Nov 2023 16:56:26 +0100 Subject: [PATCH 19/21] Readability improvement Signed-off-by: Gilles Peskine --- tests/scripts/run-metatests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/run-metatests.sh b/tests/scripts/run-metatests.sh index 09a6f8a4f..22a302c62 100755 --- a/tests/scripts/run-metatests.sh +++ b/tests/scripts/run-metatests.sh @@ -46,7 +46,7 @@ shift $((OPTIND - 1)) list_matches () { while read name platform junk; do - for pattern; do + for pattern in "$@"; do case $platform in $pattern) echo "$name"; break;; esac From 96c87c4e7bc1552389289c6697a436ad66ce3408 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 16 Nov 2023 15:09:48 +0100 Subject: [PATCH 20/21] Uniformly use MBEDTLS_THREADING_C guards Since the code compiles with MBEDTLS_THREADING_C, not just with MBEDTLS_THREADING_PTHREAD, use MBEDTLS_THREADING_C as the guard. The runtime behavior is only as desired under certain conditions that imply MBEDTLS_THREADING_PTHREAD, but that's fine: no metatest is expected to pass in all scenarios, only under specific build- and run-time conditions. Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index c35e9a952..68e8da6fa 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -141,7 +141,7 @@ void mutex_lock_not_initialized(const char *name) (void) name; /* Mutex usage verification is only done with pthread, not with other * threading implementations. See tests/src/threading_helpers.c. */ -#if defined(MBEDTLS_THREADING_PTHREAD) +#if defined(MBEDTLS_THREADING_C) mbedtls_threading_mutex_t mutex; memset(&mutex, 0, sizeof(mutex)); TEST_ASSERT(mbedtls_mutex_lock(&mutex) == 0); @@ -203,7 +203,7 @@ void mutex_leak(const char *name) (void) name; /* Mutex usage verification is only done with pthread, not with other * threading implementations. See tests/src/threading_helpers.c. */ -#if defined(MBEDTLS_THREADING_PTHREAD) +#if defined(MBEDTLS_THREADING_C) mbedtls_threading_mutex_t mutex; mbedtls_mutex_init(&mutex); #endif From e00255c41c0486a9ac3e6fda852609e2a3983347 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 16 Nov 2023 15:11:44 +0100 Subject: [PATCH 21/21] Improve explanations of what bad thing a metatest does Especially clarify the situation with respect to mutex usage. Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 47 ++++++++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index 68e8da6fa..2973cce3f 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -74,6 +74,7 @@ void null_pointer_dereference(const char *name) (void) name; volatile char *volatile p; set_to_zero_but_the_compiler_does_not_know(&p, sizeof(p)); + /* Undefined behavior (read from null data pointer) */ mbedtls_printf("%p -> %u\n", p, (unsigned) *p); } @@ -82,6 +83,7 @@ void null_pointer_call(const char *name) (void) name; unsigned(*volatile p)(void); set_to_zero_but_the_compiler_does_not_know(&p, sizeof(p)); + /* Undefined behavior (execute null function pointer) */ /* The pointer representation may be truncated, but we don't care: * the only point of printing it is to have some use of the pointer * to dissuade the compiler from optimizing it away. */ @@ -99,6 +101,7 @@ void read_after_free(const char *name) volatile char *p = mbedtls_calloc(1, 1); *p = 'a'; mbedtls_free((void *) p); + /* Undefined behavior (read after free) */ mbedtls_printf("%u\n", (unsigned) *p); } @@ -108,6 +111,7 @@ void double_free(const char *name) volatile char *p = mbedtls_calloc(1, 1); *p = 'a'; mbedtls_free((void *) p); + /* Undefined behavior (double free) */ mbedtls_free((void *) p); } @@ -120,6 +124,7 @@ void read_uninitialized_stack(const char *name) } char *volatile p = buf; if (*p != 0) { + /* Unspecified result (read from uninitialized memory) */ mbedtls_printf("%u\n", (unsigned) *p); } } @@ -129,6 +134,7 @@ void memory_leak(const char *name) (void) name; volatile char *p = mbedtls_calloc(1, 1); mbedtls_printf("%u\n", (unsigned) *p); + /* Leak of a heap object */ } @@ -139,11 +145,13 @@ void memory_leak(const char *name) void mutex_lock_not_initialized(const char *name) { (void) name; - /* Mutex usage verification is only done with pthread, not with other - * threading implementations. See tests/src/threading_helpers.c. */ #if defined(MBEDTLS_THREADING_C) mbedtls_threading_mutex_t mutex; memset(&mutex, 0, sizeof(mutex)); + /* This mutex usage error is detected by our test framework's mutex usage + * verification framework. See tests/src/threading_helpers.c. Other + * threading implementations (e.g. pthread without our instrumentation) + * might consider this normal usage. */ TEST_ASSERT(mbedtls_mutex_lock(&mutex) == 0); exit: ; @@ -153,11 +161,13 @@ exit: void mutex_unlock_not_initialized(const char *name) { (void) name; - /* Mutex usage verification is only done with pthread, not with other - * threading implementations. See tests/src/threading_helpers.c. */ #if defined(MBEDTLS_THREADING_C) mbedtls_threading_mutex_t mutex; memset(&mutex, 0, sizeof(mutex)); + /* This mutex usage error is detected by our test framework's mutex usage + * verification framework. See tests/src/threading_helpers.c. Other + * threading implementations (e.g. pthread without our instrumentation) + * might consider this normal usage. */ TEST_ASSERT(mbedtls_mutex_unlock(&mutex) == 0); exit: ; @@ -167,11 +177,13 @@ exit: void mutex_free_not_initialized(const char *name) { (void) name; - /* Mutex usage verification is only done with pthread, not with other - * threading implementations. See tests/src/threading_helpers.c. */ #if defined(MBEDTLS_THREADING_C) mbedtls_threading_mutex_t mutex; memset(&mutex, 0, sizeof(mutex)); + /* This mutex usage error is detected by our test framework's mutex usage + * verification framework. See tests/src/threading_helpers.c. Other + * threading implementations (e.g. pthread without our instrumentation) + * might consider this normal usage. */ mbedtls_mutex_free(&mutex); #endif } @@ -182,6 +194,10 @@ void mutex_double_init(const char *name) #if defined(MBEDTLS_THREADING_C) mbedtls_threading_mutex_t mutex; mbedtls_mutex_init(&mutex); + /* This mutex usage error is detected by our test framework's mutex usage + * verification framework. See tests/src/threading_helpers.c. Other + * threading implementations (e.g. pthread without our instrumentation) + * might consider this normal usage. */ mbedtls_mutex_init(&mutex); mbedtls_mutex_free(&mutex); #endif @@ -194,6 +210,10 @@ void mutex_double_free(const char *name) mbedtls_threading_mutex_t mutex; mbedtls_mutex_init(&mutex); mbedtls_mutex_free(&mutex); + /* This mutex usage error is detected by our test framework's mutex usage + * verification framework. See tests/src/threading_helpers.c. Other + * threading implementations (e.g. pthread without our instrumentation) + * might consider this normal usage. */ mbedtls_mutex_free(&mutex); #endif } @@ -201,12 +221,14 @@ void mutex_double_free(const char *name) void mutex_leak(const char *name) { (void) name; - /* Mutex usage verification is only done with pthread, not with other - * threading implementations. See tests/src/threading_helpers.c. */ #if defined(MBEDTLS_THREADING_C) mbedtls_threading_mutex_t mutex; mbedtls_mutex_init(&mutex); #endif + /* This mutex usage error is detected by our test framework's mutex usage + * verification framework. See tests/src/threading_helpers.c. Other + * threading implementations (e.g. pthread without our instrumentation) + * might consider this normal usage. */ } @@ -225,7 +247,9 @@ typedef struct { * - "any": should work anywhere. * - "asan": triggers ASan (Address Sanitizer). * - "msan": triggers MSan (Memory Sanitizer). - * - "pthread": requires MBEDTLS_THREADING_PTHREAD and MBEDTLS_TEST_HOOKS. + * - "pthread": requires MBEDTLS_THREADING_PTHREAD and MBEDTLS_TEST_HOOKS, + * which enables MBEDTLS_TEST_MUTEX_USAGE internally in the test + * framework (see tests/src/threading_helpers.c). */ const char *platform; @@ -249,6 +273,9 @@ typedef struct { * * Note that we always compile all the functions, so that `metatest --list` * will always list all the available meta-tests. + * + * See the documentation of metatest_t::platform for the meaning of + * platform values. */ metatest_t metatests[] = { { "test_fail", "any", meta_test_fail }, @@ -258,8 +285,6 @@ metatest_t metatests[] = { { "double_free", "asan", double_free }, { "read_uninitialized_stack", "msan", read_uninitialized_stack }, { "memory_leak", "asan", memory_leak }, - /* Mutex usage verification is only done with pthread, not with other - * threading implementations. See tests/src/threading_helpers.c. */ { "mutex_lock_not_initialized", "pthread", mutex_lock_not_initialized }, { "mutex_unlock_not_initialized", "pthread", mutex_unlock_not_initialized }, { "mutex_free_not_initialized", "pthread", mutex_free_not_initialized },