From 756b4dcfa4859fd362ebd276cb1241719a93443a Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Wed, 10 Jan 2024 14:33:17 +0000 Subject: [PATCH 1/6] 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 43fbb6350f..0bf8f469d5 100644 --- a/tests/include/test/memory.h +++ b/tests/include/test/memory.h @@ -21,9 +21,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 @@ -61,6 +64,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. * @@ -68,7 +77,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(). * @@ -79,7 +91,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 d074a5a14709248b46c0d5c40ea73791735ee8a5 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Thu, 11 Jan 2024 16:33:46 +0000 Subject: [PATCH 2/6] 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 1724aed0bd..6fcf52a505 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -366,6 +366,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 }, @@ -382,6 +383,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 6de58282888033f08efc7eff06193e3426af0a68 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Wed, 17 Jan 2024 14:23:20 +0000 Subject: [PATCH 3/6] 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 0bf8f469d5..6d0f76478a 100644 --- a/tests/include/test/memory.h +++ b/tests/include/test/memory.h @@ -68,7 +68,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. @@ -78,7 +78,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) @@ -93,7 +93,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 1b421b1005ccadf8678cf8b102c105c9eb86cffe Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Wed, 17 Jan 2024 14:53:08 +0000 Subject: [PATCH 4/6] 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 6fcf52a505..a082621915 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -366,24 +366,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 02513e903a..81be111fd5 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -876,7 +876,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 @@ -1497,7 +1497,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 e7bfbc27bf0cbaf181b741dcf42a36758dd17840 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Wed, 17 Jan 2024 15:27:50 +0000 Subject: [PATCH 5/6] 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 6d0f76478a..91be573b9b 100644 --- a/tests/include/test/memory.h +++ b/tests/include/test/memory.h @@ -93,7 +93,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 */ From 7dfb6121fc5b7fb8d088f67a46519795486c744d Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 23 Jan 2024 15:35:20 +0000 Subject: [PATCH 6/6] Remove _Thread_local variable for 2.28 We do not intend to support multithreaded testing in 2.28, so introducing a C11 feature here is an unnecessary burden. Signed-off-by: David Horstmann --- tests/include/test/memory.h | 13 +++++-------- tests/src/test_memory.c | 2 +- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/tests/include/test/memory.h b/tests/include/test/memory.h index 91be573b9b..d4bbeec0d4 100644 --- a/tests/include/test/memory.h +++ b/tests/include/test/memory.h @@ -21,12 +21,9 @@ * 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) && \ - (__STDC_VERSION__ >= 201112L) +#if defined(MBEDTLS_TEST_HAVE_ASAN) # define MBEDTLS_TEST_MEMORY_CAN_POISON #endif @@ -64,11 +61,11 @@ #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. +/** 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 unsigned int mbedtls_test_memory_poisoning_count; +extern 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. diff --git a/tests/src/test_memory.c b/tests/src/test_memory.c index ac9dde6163..9da7f20a36 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 unsigned int mbedtls_test_memory_poisoning_count = 0; +unsigned int mbedtls_test_memory_poisoning_count = 0; static void align_for_asan(const unsigned char **p_ptr, size_t *p_size) {