From 8ba3f68561224c0b6b2ac34822ef45ab7895821d Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Thu, 11 Jan 2024 15:53:52 +0000 Subject: [PATCH 1/7] Stop enforcing C99 in CMakeLists.txt This allows us to sometimes use C11 features. Signed-off-by: David Horstmann --- CMakeLists.txt | 3 --- 1 file changed, 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 36baa3b402..210cc38f34 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -177,9 +177,6 @@ string(REGEX MATCH "Clang" CMAKE_COMPILER_IS_CLANG "${CMAKE_C_COMPILER_ID}") include(CheckCCompilerFlag) -set(CMAKE_C_EXTENSIONS OFF) -set(CMAKE_C_STANDARD 99) - if(CMAKE_COMPILER_IS_GNU) # some warnings we want are not available with old GCC versions # note: starting with CMake 2.8 we could use CMAKE_C_COMPILER_VERSION From 8174478f82c6e78a008cd266212b404528a3207a Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Wed, 10 Jan 2024 14:33:17 +0000 Subject: [PATCH 2/7] Use thread-local flag to enable memory poisoning Allow memory poisoning to be enabled and disabled at runtime using a thread-local flag. This allows poisoning to be disabled whenever a PSA function is called but not through the test wrappers, removing false positive use-after-poisons. Signed-off-by: David Horstmann --- tests/include/test/memory.h | 21 ++++++++++++++++++--- tests/src/test_memory.c | 12 +++++++++--- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/tests/include/test/memory.h b/tests/include/test/memory.h index c22ef53f7c..e0437517da 100644 --- a/tests/include/test/memory.h +++ b/tests/include/test/memory.h @@ -22,9 +22,12 @@ * memory as poisoned, which can be used to enforce some memory access * policies. * + * Support for the C11 thread_local keyword is also required. + * * Currently, only Asan (Address Sanitizer) is supported. */ -#if defined(MBEDTLS_TEST_HAVE_ASAN) +#if defined(MBEDTLS_TEST_HAVE_ASAN) && \ + (__STDC_VERSION__ >= 201112L) # define MBEDTLS_TEST_MEMORY_CAN_POISON #endif @@ -62,6 +65,12 @@ #if defined(MBEDTLS_TEST_MEMORY_CAN_POISON) +/** Thread-local variable used to enable memory poisoning. This is set and + * unset in the test wrappers so that calls to PSA functions from the library + * do not poison memory. + */ +extern _Thread_local int mbedtls_test_memory_poisoning_enabled; + /** Poison a memory area so that any attempt to read or write from it will * cause a runtime failure. * @@ -69,7 +78,10 @@ */ void mbedtls_test_memory_poison(const unsigned char *ptr, size_t size); #define MBEDTLS_TEST_MEMORY_POISON(ptr, size) \ - mbedtls_test_memory_poison(ptr, size) + do { \ + mbedtls_test_memory_poisoning_enabled = 1; \ + mbedtls_test_memory_poison(ptr, size); \ + } while (0) /** Undo the effect of mbedtls_test_memory_poison(). * @@ -80,7 +92,10 @@ void mbedtls_test_memory_poison(const unsigned char *ptr, size_t size); */ void mbedtls_test_memory_unpoison(const unsigned char *ptr, size_t size); #define MBEDTLS_TEST_MEMORY_UNPOISON(ptr, size) \ - mbedtls_test_memory_unpoison(ptr, size) + do { \ + mbedtls_test_memory_unpoison(ptr, size); \ + mbedtls_test_memory_poisoning_enabled = 0; \ + } while (0) #else /* MBEDTLS_TEST_MEMORY_CAN_POISON */ #define MBEDTLS_TEST_MEMORY_POISON(ptr, size) ((void) (ptr), (void) (size)) diff --git a/tests/src/test_memory.c b/tests/src/test_memory.c index c277be85ab..dbb7b20de2 100644 --- a/tests/src/test_memory.c +++ b/tests/src/test_memory.c @@ -13,12 +13,15 @@ #include #include -#if defined(MBEDTLS_TEST_HAVE_ASAN) +#if defined(MBEDTLS_TEST_MEMORY_CAN_POISON) #include #include #endif -#if defined(MBEDTLS_TEST_HAVE_ASAN) +#if defined(MBEDTLS_TEST_MEMORY_CAN_POISON) + +_Thread_local int mbedtls_test_memory_poisoning_enabled = 0; + static void align_for_asan(const unsigned char **p_ptr, size_t *p_size) { uintptr_t start = (uintptr_t) *p_ptr; @@ -36,6 +39,9 @@ static void align_for_asan(const unsigned char **p_ptr, size_t *p_size) void mbedtls_test_memory_poison(const unsigned char *ptr, size_t size) { + if (!mbedtls_test_memory_poisoning_enabled) { + return; + } if (size == 0) { return; } @@ -51,4 +57,4 @@ void mbedtls_test_memory_unpoison(const unsigned char *ptr, size_t size) align_for_asan(&ptr, &size); __asan_unpoison_memory_region(ptr, size); } -#endif /* Asan */ +#endif /* Memory poisoning */ From 9432e64933f77fb3392b9346607670ea7caaacd6 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Thu, 11 Jan 2024 16:33:46 +0000 Subject: [PATCH 3/7] Only run memory poisoning metatests when poisoning When we cannot memory poison due to platform constraints, do not attempt to run memory poisoning metatests (but still run other ASan metatests). Signed-off-by: David Horstmann --- programs/test/metatest.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index 392b638420..bdcb1df9af 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -362,6 +362,7 @@ metatest_t metatests[] = { { "double_free", "asan", double_free }, { "read_uninitialized_stack", "msan", read_uninitialized_stack }, { "memory_leak", "asan", memory_leak }, +#if defined(MBEDTLS_TEST_MEMORY_CAN_POISON) { "test_memory_poison_0_0_8_r", "asan", test_memory_poison }, { "test_memory_poison_0_0_8_w", "asan", test_memory_poison }, { "test_memory_poison_0_7_8_r", "asan", test_memory_poison }, @@ -378,6 +379,7 @@ metatest_t metatests[] = { { "test_memory_poison_7_0_1_w", "asan", test_memory_poison }, { "test_memory_poison_7_1_2_r", "asan", test_memory_poison }, { "test_memory_poison_7_1_2_w", "asan", test_memory_poison }, +#endif /* MBEDTLS_TEST_MEMORY_CAN_POISON */ { "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 }, From c2ab398d01cf7bec9fda5796c08cc3043c71a570 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Wed, 17 Jan 2024 12:25:19 +0000 Subject: [PATCH 4/7] Request C11 in CMake (but only for tests) Set the C_STANDARD property on the mbedtls_test target to 11. This requests C11 for the tests only. If C11 is not supported the build will not fail, since C_STANDARD_REQUIRED is not set, and memory poisoning will be disabled by a preprocessor check on __STDC_VERSION__. Additionally, reintroduce previous C99 enforcement on the rest of the library. Signed-off-by: David Horstmann --- CMakeLists.txt | 5 +++++ programs/test/CMakeLists.txt | 3 +++ tests/CMakeLists.txt | 2 ++ 3 files changed, 10 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 210cc38f34..0cb208e988 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -177,6 +177,9 @@ string(REGEX MATCH "Clang" CMAKE_COMPILER_IS_CLANG "${CMAKE_C_COMPILER_ID}") include(CheckCCompilerFlag) +set(CMAKE_C_EXTENSIONS OFF) +set(CMAKE_C_STANDARD 99) + if(CMAKE_COMPILER_IS_GNU) # some warnings we want are not available with old GCC versions # note: starting with CMake 2.8 we could use CMAKE_C_COMPILER_VERSION @@ -294,6 +297,8 @@ if(ENABLE_TESTING OR ENABLE_PROGRAMS) PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/tests/include PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/library) + # Request C11, needed for memory poisoning tests + set_target_properties(mbedtls_test PROPERTIES C_STANDARD 11) file(GLOB MBEDTLS_TEST_HELPER_FILES ${CMAKE_CURRENT_SOURCE_DIR}/tests/src/test_helpers/*.c) diff --git a/programs/test/CMakeLists.txt b/programs/test/CMakeLists.txt index 0778731125..5a26821b51 100644 --- a/programs/test/CMakeLists.txt +++ b/programs/test/CMakeLists.txt @@ -78,6 +78,9 @@ foreach(exe IN LISTS executables_libs executables_mbedcrypto) target_include_directories(${exe} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) endif() + # Request C11, required for memory poisoning + set_target_properties(${exe} PROPERTIES C_STANDARD 11) + # This emulates "if ( ... IN_LIST ... )" which becomes available in CMake 3.3 list(FIND executables_libs ${exe} exe_index) if (${exe_index} GREATER -1) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0869aaa018..188d5d595f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -251,6 +251,8 @@ function(add_test_suite suite_name) target_include_directories(test_suite_${data_name} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../library) + # Request C11, which is needed for memory poisoning tests + set_target_properties(test_suite_${data_name} PROPERTIES C_STANDARD 11) if(${data_name} MATCHES ${SKIP_TEST_SUITES_REGEX}) message(STATUS "The test suite ${data_name} will not be executed.") From fad038c5015b4667fd343361702b6ff675a132b1 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Wed, 17 Jan 2024 14:23:20 +0000 Subject: [PATCH 5/7] Change memory poisoning flag to a count This allows unusually-nested memory poisoning to work correctly, since it keeps track of whether any buffers are still poisoned, rather than just disabling poisoning at the first call to the UNPOISON() macro. Signed-off-by: David Horstmann --- tests/include/test/memory.h | 6 +++--- tests/src/test_memory.c | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/include/test/memory.h b/tests/include/test/memory.h index e0437517da..181280f265 100644 --- a/tests/include/test/memory.h +++ b/tests/include/test/memory.h @@ -69,7 +69,7 @@ * unset in the test wrappers so that calls to PSA functions from the library * do not poison memory. */ -extern _Thread_local int mbedtls_test_memory_poisoning_enabled; +extern _Thread_local unsigned int mbedtls_test_memory_poisoning_count; /** Poison a memory area so that any attempt to read or write from it will * cause a runtime failure. @@ -79,7 +79,7 @@ extern _Thread_local int mbedtls_test_memory_poisoning_enabled; void mbedtls_test_memory_poison(const unsigned char *ptr, size_t size); #define MBEDTLS_TEST_MEMORY_POISON(ptr, size) \ do { \ - mbedtls_test_memory_poisoning_enabled = 1; \ + mbedtls_test_memory_poisoning_count++; \ mbedtls_test_memory_poison(ptr, size); \ } while (0) @@ -94,7 +94,7 @@ void mbedtls_test_memory_unpoison(const unsigned char *ptr, size_t size); #define MBEDTLS_TEST_MEMORY_UNPOISON(ptr, size) \ do { \ mbedtls_test_memory_unpoison(ptr, size); \ - mbedtls_test_memory_poisoning_enabled = 0; \ + mbedtls_test_memory_poisoning_count--; \ } while (0) #else /* MBEDTLS_TEST_MEMORY_CAN_POISON */ diff --git a/tests/src/test_memory.c b/tests/src/test_memory.c index dbb7b20de2..ac9dde6163 100644 --- a/tests/src/test_memory.c +++ b/tests/src/test_memory.c @@ -20,7 +20,7 @@ #if defined(MBEDTLS_TEST_MEMORY_CAN_POISON) -_Thread_local int mbedtls_test_memory_poisoning_enabled = 0; +_Thread_local unsigned int mbedtls_test_memory_poisoning_count = 0; static void align_for_asan(const unsigned char **p_ptr, size_t *p_size) { @@ -39,7 +39,7 @@ static void align_for_asan(const unsigned char **p_ptr, size_t *p_size) void mbedtls_test_memory_poison(const unsigned char *ptr, size_t size) { - if (!mbedtls_test_memory_poisoning_enabled) { + if (mbedtls_test_memory_poisoning_count == 0) { return; } if (size == 0) { From 9de6edd46246dbf2e42f449ad3a6fa37d1b6155e Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Wed, 17 Jan 2024 14:53:08 +0000 Subject: [PATCH 6/7] Separate memory poisoning tests from generic ASan Some platforms may support ASan but be C99-only (no C11 support). These platforms will support ASan metatests but not memory poisoning, which requires C11 features. To allow for this, create a separate platform requirement, "poison", in metatest.c to distinguish generic ASan metatests from ones that require suppport for memory poisoning. In practice our platforms support both, so run "poison" tests in the same all.sh components where we run "asan" ones. Signed-off-by: David Horstmann --- programs/test/metatest.c | 34 ++++++++++++++++------------------ tests/scripts/all.sh | 4 ++-- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index bdcb1df9af..79c57d1746 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -362,24 +362,22 @@ metatest_t metatests[] = { { "double_free", "asan", double_free }, { "read_uninitialized_stack", "msan", read_uninitialized_stack }, { "memory_leak", "asan", memory_leak }, -#if defined(MBEDTLS_TEST_MEMORY_CAN_POISON) - { "test_memory_poison_0_0_8_r", "asan", test_memory_poison }, - { "test_memory_poison_0_0_8_w", "asan", test_memory_poison }, - { "test_memory_poison_0_7_8_r", "asan", test_memory_poison }, - { "test_memory_poison_0_7_8_w", "asan", test_memory_poison }, - { "test_memory_poison_0_0_1_r", "asan", test_memory_poison }, - { "test_memory_poison_0_0_1_w", "asan", test_memory_poison }, - { "test_memory_poison_0_1_2_r", "asan", test_memory_poison }, - { "test_memory_poison_0_1_2_w", "asan", test_memory_poison }, - { "test_memory_poison_7_0_8_r", "asan", test_memory_poison }, - { "test_memory_poison_7_0_8_w", "asan", test_memory_poison }, - { "test_memory_poison_7_7_8_r", "asan", test_memory_poison }, - { "test_memory_poison_7_7_8_w", "asan", test_memory_poison }, - { "test_memory_poison_7_0_1_r", "asan", test_memory_poison }, - { "test_memory_poison_7_0_1_w", "asan", test_memory_poison }, - { "test_memory_poison_7_1_2_r", "asan", test_memory_poison }, - { "test_memory_poison_7_1_2_w", "asan", test_memory_poison }, -#endif /* MBEDTLS_TEST_MEMORY_CAN_POISON */ + { "test_memory_poison_0_0_8_r", "poison", test_memory_poison }, + { "test_memory_poison_0_0_8_w", "poison", test_memory_poison }, + { "test_memory_poison_0_7_8_r", "poison", test_memory_poison }, + { "test_memory_poison_0_7_8_w", "poison", test_memory_poison }, + { "test_memory_poison_0_0_1_r", "poison", test_memory_poison }, + { "test_memory_poison_0_0_1_w", "poison", test_memory_poison }, + { "test_memory_poison_0_1_2_r", "poison", test_memory_poison }, + { "test_memory_poison_0_1_2_w", "poison", test_memory_poison }, + { "test_memory_poison_7_0_8_r", "poison", test_memory_poison }, + { "test_memory_poison_7_0_8_w", "poison", test_memory_poison }, + { "test_memory_poison_7_7_8_r", "poison", test_memory_poison }, + { "test_memory_poison_7_7_8_w", "poison", test_memory_poison }, + { "test_memory_poison_7_0_1_r", "poison", test_memory_poison }, + { "test_memory_poison_7_0_1_w", "poison", test_memory_poison }, + { "test_memory_poison_7_1_2_r", "poison", test_memory_poison }, + { "test_memory_poison_7_1_2_w", "poison", test_memory_poison }, { "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 }, diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 4465d05068..0daab0918d 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1125,7 +1125,7 @@ component_test_default_cmake_gcc_asan () { programs/test/selftest msg "test: metatests (GCC, ASan build)" - tests/scripts/run-metatests.sh any asan + tests/scripts/run-metatests.sh any asan poison msg "test: ssl-opt.sh (ASan build)" # ~ 1 min tests/ssl-opt.sh @@ -1944,7 +1944,7 @@ component_test_everest () { make test msg "test: metatests (clang, ASan)" - tests/scripts/run-metatests.sh any asan + tests/scripts/run-metatests.sh any asan poison msg "test: Everest ECDH context - ECDH-related part of ssl-opt.sh (ASan build)" # ~ 5s tests/ssl-opt.sh -f ECDH From d3efb92922e4e897563b028974ee40818fdeee99 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Wed, 17 Jan 2024 15:27:50 +0000 Subject: [PATCH 7/7] Add underflow check to UNPOISON counter decrement Make sure that extra UNPOISON calls do not cause the poisoning counter to underflow and wrap around. Memory that is unpoisoned multiple times should remain unpoisoned. Signed-off-by: David Horstmann --- tests/include/test/memory.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/include/test/memory.h b/tests/include/test/memory.h index 181280f265..20fd8d30a5 100644 --- a/tests/include/test/memory.h +++ b/tests/include/test/memory.h @@ -94,7 +94,9 @@ void mbedtls_test_memory_unpoison(const unsigned char *ptr, size_t size); #define MBEDTLS_TEST_MEMORY_UNPOISON(ptr, size) \ do { \ mbedtls_test_memory_unpoison(ptr, size); \ - mbedtls_test_memory_poisoning_count--; \ + if (mbedtls_test_memory_poisoning_count != 0) { \ + mbedtls_test_memory_poisoning_count--; \ + } \ } while (0) #else /* MBEDTLS_TEST_MEMORY_CAN_POISON */