From 2906014a4c939d6cccb47848cfe6b9af32a5ccfa Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 31 Aug 2025 18:15:47 +0200 Subject: [PATCH 1/4] Support mutex API functions not being the mutable pointers Signed-off-by: Gilles Peskine --- tests/src/threading_helpers.c | 38 +++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/tests/src/threading_helpers.c b/tests/src/threading_helpers.c index c1686c21a..347f8eadc 100644 --- a/tests/src/threading_helpers.c +++ b/tests/src/threading_helpers.c @@ -9,10 +9,22 @@ #include #include -#include "mbedtls/threading.h" +#include "threading_internal.h" #if defined(MBEDTLS_THREADING_C) +#if MBEDTLS_THREADING_INTERNAL_VERSION < 0x04000000 +/* Historically, the mutex functions in the API were function pointers. + * Since TF-PSA-Crypto 1.0.0 (paired with Mbed TLS 4.0.0), the API + * functions have the historical names but the pointers have different + * names. When building against Mbed TLS 3.6.x, define the pointer name + * as aliases. */ +#define mbedtls_mutex_init_ptr mbedtls_mutex_init +#define mbedtls_mutex_free_ptr mbedtls_mutex_free +#define mbedtls_mutex_lock_ptr mbedtls_mutex_lock +#define mbedtls_mutex_unlock_ptr mbedtls_mutex_unlock +#endif + #if defined(MBEDTLS_THREADING_PTHREAD) static int threading_thread_create_pthread(mbedtls_test_thread_t *thread, void *(*thread_func)( @@ -303,14 +315,14 @@ static int mbedtls_test_wrap_mutex_unlock(mbedtls_threading_mutex_t *mutex) void mbedtls_test_mutex_usage_init(void) { - mutex_functions.init = mbedtls_mutex_init; - mutex_functions.free = mbedtls_mutex_free; - mutex_functions.lock = mbedtls_mutex_lock; - mutex_functions.unlock = mbedtls_mutex_unlock; - mbedtls_mutex_init = &mbedtls_test_wrap_mutex_init; - mbedtls_mutex_free = &mbedtls_test_wrap_mutex_free; - mbedtls_mutex_lock = &mbedtls_test_wrap_mutex_lock; - mbedtls_mutex_unlock = &mbedtls_test_wrap_mutex_unlock; + mutex_functions.init = *mbedtls_mutex_init_ptr; + mutex_functions.free = *mbedtls_mutex_free_ptr; + mutex_functions.lock = *mbedtls_mutex_lock_ptr; + mutex_functions.unlock = *mbedtls_mutex_unlock_ptr; + mbedtls_mutex_init_ptr = &mbedtls_test_wrap_mutex_init; + mbedtls_mutex_free_ptr = &mbedtls_test_wrap_mutex_free; + mbedtls_mutex_lock_ptr = &mbedtls_test_wrap_mutex_lock; + mbedtls_mutex_unlock_ptr = &mbedtls_test_wrap_mutex_unlock; mutex_functions.init(&mbedtls_test_mutex_mutex); } @@ -341,10 +353,10 @@ void mbedtls_test_mutex_usage_check(void) void mbedtls_test_mutex_usage_end(void) { - mbedtls_mutex_init = mutex_functions.init; - mbedtls_mutex_free = mutex_functions.free; - mbedtls_mutex_lock = mutex_functions.lock; - mbedtls_mutex_unlock = mutex_functions.unlock; + mbedtls_mutex_init_ptr = mutex_functions.init; + mbedtls_mutex_free_ptr = mutex_functions.free; + mbedtls_mutex_lock_ptr = mutex_functions.lock; + mbedtls_mutex_unlock_ptr = mutex_functions.unlock; mutex_functions.free(&mbedtls_test_mutex_mutex); } From 34bd04d6dd47f956d2fa08fe26d6abd3d22d8783 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 31 Aug 2025 18:39:54 +0200 Subject: [PATCH 2/4] Support mutex API type being separate from the platform type The way the mutex usage verification framework plugs into the library requires it to have access to the API type, which holds the state field. In the new threading API, the mutable function that the framework overrides now only receives a pointer to the platform object that is embedded in the API object. Hence we need to calculate a pointer to the containing API object when given a pointer to the platform object. It's ugly, but it works. A follow-up should clean this up by changing how the mutex usage verification framework plugs into the library. It should use a more normal hook function mechanism instead of replacing the function pointers. Signed-off-by: Gilles Peskine --- tests/src/threading_helpers.c | 65 ++++++++++++++++++++++++----------- 1 file changed, 45 insertions(+), 20 deletions(-) diff --git a/tests/src/threading_helpers.c b/tests/src/threading_helpers.c index 347f8eadc..f4a29f02f 100644 --- a/tests/src/threading_helpers.c +++ b/tests/src/threading_helpers.c @@ -23,7 +23,32 @@ #define mbedtls_mutex_free_ptr mbedtls_mutex_free #define mbedtls_mutex_lock_ptr mbedtls_mutex_lock #define mbedtls_mutex_unlock_ptr mbedtls_mutex_unlock -#endif + +typedef mbedtls_threading_mutex_t mbedtls_platform_mutex_t; +#define mutex_container(platform_mutex) (platform_mutex) + +#else /* MBEDTLS_THREADING_INTERNAL_VERSION >= 0x04000000 */ + +/* Historically, the mutex platform functions received a pointer to the + * mbedtls_threading_mutex_t object, and the pthread implementation of + * that type had a state field. Now the platform functions receive a pointer + * to the mbedtls_platform_mutex_t object which is a field of the + * mbedtls_threading_mutex_t object. Get a pointer to the containing + * object which holds the state field. + * + * This weird arrangement was done to minimize changes when switching the + * mutex usage framework to the separated platform/API types. In the future + * we should clean up how the usage framework fits into the library. + */ +static mbedtls_threading_mutex_t *mutex_container( + mbedtls_platform_mutex_t *platform_mutex) +{ + unsigned char *field_address = (unsigned char *) platform_mutex; + size_t offset = offsetof(mbedtls_threading_mutex_t, mutex); + return (mbedtls_threading_mutex_t *) (field_address - offset); +} + +#endif /* MBEDTLS_THREADING_INTERNAL_VERSION */ #if defined(MBEDTLS_THREADING_PTHREAD) @@ -148,10 +173,10 @@ enum value_of_mutex_state_field { }; typedef struct { - void (*init)(mbedtls_threading_mutex_t *); - void (*free)(mbedtls_threading_mutex_t *); - int (*lock)(mbedtls_threading_mutex_t *); - int (*unlock)(mbedtls_threading_mutex_t *); + void (*init)(mbedtls_platform_mutex_t *); + void (*free)(mbedtls_platform_mutex_t *); + int (*lock)(mbedtls_platform_mutex_t *); + int (*unlock)(mbedtls_platform_mutex_t *); } mutex_functions_t; static mutex_functions_t mutex_functions; @@ -171,7 +196,7 @@ static mutex_functions_t mutex_functions; * testing. This is not a situation that is likely to happen with normal * testing and we still have TSan to fall back on should this happen. */ -mbedtls_threading_mutex_t mbedtls_test_mutex_mutex; +mbedtls_platform_mutex_t mbedtls_test_mutex_mutex; /** * The total number of calls to mbedtls_mutex_init(), minus the total number @@ -182,7 +207,7 @@ mbedtls_threading_mutex_t mbedtls_test_mutex_mutex; */ static int live_mutexes; -static void mbedtls_test_mutex_usage_error(mbedtls_threading_mutex_t *mutex, +static void mbedtls_test_mutex_usage_error(mbedtls_platform_mutex_t *mutex, const char *msg) { (void) mutex; @@ -195,7 +220,7 @@ static void mbedtls_test_mutex_usage_error(mbedtls_threading_mutex_t *mutex, * mbedtls_test_mutex_usage_check() will mark it as failed. */ } -static int mbedtls_test_mutex_can_test(mbedtls_threading_mutex_t *mutex) +static int mbedtls_test_mutex_can_test(mbedtls_platform_mutex_t *mutex) { /* If we attempt to run tests on this mutex then we are going to run into a * couple of problems: @@ -203,20 +228,20 @@ static int mbedtls_test_mutex_can_test(mbedtls_threading_mutex_t *mutex) * reporting that failure, as we already hold the mutex at that point. * 2. Given the 'global' position of the initialization and free of this * mutex, it will be shown as leaked on the first test run. */ - if (mutex == mbedtls_test_get_info_mutex()) { + if (mutex_container(mutex) == mbedtls_test_get_info_mutex()) { return 0; } return 1; } -static void mbedtls_test_wrap_mutex_init(mbedtls_threading_mutex_t *mutex) +static void mbedtls_test_wrap_mutex_init(mbedtls_platform_mutex_t *mutex) { mutex_functions.init(mutex); if (mbedtls_test_mutex_can_test(mutex)) { if (mutex_functions.lock(&mbedtls_test_mutex_mutex) == 0) { - mutex->state = MUTEX_IDLE; + mutex_container(mutex)->state = MUTEX_IDLE; ++live_mutexes; mutex_functions.unlock(&mbedtls_test_mutex_mutex); @@ -224,17 +249,17 @@ static void mbedtls_test_wrap_mutex_init(mbedtls_threading_mutex_t *mutex) } } -static void mbedtls_test_wrap_mutex_free(mbedtls_threading_mutex_t *mutex) +static void mbedtls_test_wrap_mutex_free(mbedtls_platform_mutex_t *mutex) { if (mbedtls_test_mutex_can_test(mutex)) { if (mutex_functions.lock(&mbedtls_test_mutex_mutex) == 0) { - switch (mutex->state) { + switch (mutex_container(mutex)->state) { case MUTEX_FREED: mbedtls_test_mutex_usage_error(mutex, "free without init or double free"); break; case MUTEX_IDLE: - mutex->state = MUTEX_FREED; + mutex_container(mutex)->state = MUTEX_FREED; --live_mutexes; break; case MUTEX_LOCKED: @@ -252,7 +277,7 @@ static void mbedtls_test_wrap_mutex_free(mbedtls_threading_mutex_t *mutex) mutex_functions.free(mutex); } -static int mbedtls_test_wrap_mutex_lock(mbedtls_threading_mutex_t *mutex) +static int mbedtls_test_wrap_mutex_lock(mbedtls_platform_mutex_t *mutex) { /* Lock the passed in mutex first, so that the only way to change the state * is to hold the passed in and internal mutex - otherwise we create a race @@ -261,13 +286,13 @@ static int mbedtls_test_wrap_mutex_lock(mbedtls_threading_mutex_t *mutex) if (mbedtls_test_mutex_can_test(mutex)) { if (mutex_functions.lock(&mbedtls_test_mutex_mutex) == 0) { - switch (mutex->state) { + switch (mutex_container(mutex)->state) { case MUTEX_FREED: mbedtls_test_mutex_usage_error(mutex, "lock without init"); break; case MUTEX_IDLE: if (ret == 0) { - mutex->state = MUTEX_LOCKED; + mutex_container(mutex)->state = MUTEX_LOCKED; } break; case MUTEX_LOCKED: @@ -285,14 +310,14 @@ static int mbedtls_test_wrap_mutex_lock(mbedtls_threading_mutex_t *mutex) return ret; } -static int mbedtls_test_wrap_mutex_unlock(mbedtls_threading_mutex_t *mutex) +static int mbedtls_test_wrap_mutex_unlock(mbedtls_platform_mutex_t *mutex) { /* Lock the internal mutex first and change state, so that the only way to * change the state is to hold the passed in and internal mutex - otherwise * we create a race condition. */ if (mbedtls_test_mutex_can_test(mutex)) { if (mutex_functions.lock(&mbedtls_test_mutex_mutex) == 0) { - switch (mutex->state) { + switch (mutex_container(mutex)->state) { case MUTEX_FREED: mbedtls_test_mutex_usage_error(mutex, "unlock without init"); break; @@ -300,7 +325,7 @@ static int mbedtls_test_wrap_mutex_unlock(mbedtls_threading_mutex_t *mutex) mbedtls_test_mutex_usage_error(mutex, "unlock without lock"); break; case MUTEX_LOCKED: - mutex->state = MUTEX_IDLE; + mutex_container(mutex)->state = MUTEX_IDLE; break; default: mbedtls_test_mutex_usage_error(mutex, "corrupted state"); From bf196ac2fdc4d4d75563c2e60d66b41a103d622b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 1 Sep 2025 19:23:12 +0200 Subject: [PATCH 3/4] Only define mutex usage test helpers when mutex usage tests are used Fixes an unused function warning. Signed-off-by: Gilles Peskine --- tests/src/threading_helpers.c | 74 +++++++++++++++++------------------ 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/tests/src/threading_helpers.c b/tests/src/threading_helpers.c index f4a29f02f..bbac43a75 100644 --- a/tests/src/threading_helpers.c +++ b/tests/src/threading_helpers.c @@ -13,43 +13,6 @@ #if defined(MBEDTLS_THREADING_C) -#if MBEDTLS_THREADING_INTERNAL_VERSION < 0x04000000 -/* Historically, the mutex functions in the API were function pointers. - * Since TF-PSA-Crypto 1.0.0 (paired with Mbed TLS 4.0.0), the API - * functions have the historical names but the pointers have different - * names. When building against Mbed TLS 3.6.x, define the pointer name - * as aliases. */ -#define mbedtls_mutex_init_ptr mbedtls_mutex_init -#define mbedtls_mutex_free_ptr mbedtls_mutex_free -#define mbedtls_mutex_lock_ptr mbedtls_mutex_lock -#define mbedtls_mutex_unlock_ptr mbedtls_mutex_unlock - -typedef mbedtls_threading_mutex_t mbedtls_platform_mutex_t; -#define mutex_container(platform_mutex) (platform_mutex) - -#else /* MBEDTLS_THREADING_INTERNAL_VERSION >= 0x04000000 */ - -/* Historically, the mutex platform functions received a pointer to the - * mbedtls_threading_mutex_t object, and the pthread implementation of - * that type had a state field. Now the platform functions receive a pointer - * to the mbedtls_platform_mutex_t object which is a field of the - * mbedtls_threading_mutex_t object. Get a pointer to the containing - * object which holds the state field. - * - * This weird arrangement was done to minimize changes when switching the - * mutex usage framework to the separated platform/API types. In the future - * we should clean up how the usage framework fits into the library. - */ -static mbedtls_threading_mutex_t *mutex_container( - mbedtls_platform_mutex_t *platform_mutex) -{ - unsigned char *field_address = (unsigned char *) platform_mutex; - size_t offset = offsetof(mbedtls_threading_mutex_t, mutex); - return (mbedtls_threading_mutex_t *) (field_address - offset); -} - -#endif /* MBEDTLS_THREADING_INTERNAL_VERSION */ - #if defined(MBEDTLS_THREADING_PTHREAD) static int threading_thread_create_pthread(mbedtls_test_thread_t *thread, void *(*thread_func)( @@ -172,6 +135,43 @@ enum value_of_mutex_state_field { MUTEX_LOCKED = 2, //! < Set by mbedtls_test_wrap_mutex_lock }; +#if MBEDTLS_THREADING_INTERNAL_VERSION < 0x04000000 +/* Historically, the mutex functions in the API were function pointers. + * Since TF-PSA-Crypto 1.0.0 (paired with Mbed TLS 4.0.0), the API + * functions have the historical names but the pointers have different + * names. When building against Mbed TLS 3.6.x, define the pointer name + * as aliases. */ +#define mbedtls_mutex_init_ptr mbedtls_mutex_init +#define mbedtls_mutex_free_ptr mbedtls_mutex_free +#define mbedtls_mutex_lock_ptr mbedtls_mutex_lock +#define mbedtls_mutex_unlock_ptr mbedtls_mutex_unlock + +typedef mbedtls_threading_mutex_t mbedtls_platform_mutex_t; +#define mutex_container(platform_mutex) (platform_mutex) + +#else /* MBEDTLS_THREADING_INTERNAL_VERSION >= 0x04000000 */ + +/* Historically, the mutex platform functions received a pointer to the + * mbedtls_threading_mutex_t object, and the pthread implementation of + * that type had a state field. Now the platform functions receive a pointer + * to the mbedtls_platform_mutex_t object which is a field of the + * mbedtls_threading_mutex_t object. Get a pointer to the containing + * object which holds the state field. + * + * This weird arrangement was done to minimize changes when switching the + * mutex usage framework to the separated platform/API types. In the future + * we should clean up how the usage framework fits into the library. + */ +static mbedtls_threading_mutex_t *mutex_container( + mbedtls_platform_mutex_t *platform_mutex) +{ + unsigned char *field_address = (unsigned char *) platform_mutex; + size_t offset = offsetof(mbedtls_threading_mutex_t, mutex); + return (mbedtls_threading_mutex_t *) (field_address - offset); +} + +#endif /* MBEDTLS_THREADING_INTERNAL_VERSION */ + typedef struct { void (*init)(mbedtls_platform_mutex_t *); void (*free)(mbedtls_platform_mutex_t *); From 8ef6acd6273073764c297ee0111220293cb37e23 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 2 Sep 2025 00:16:32 +0200 Subject: [PATCH 4/4] Define MBEDTLS_ERR_THREADING_BAD_INPUT_DATA for TF-PSA-Crypto In Mbed TLS 3.6, this error was used in the library as well as the test framework, but since TF-PSA-Crypto 1.0, it has been removed from the library. Signed-off-by: Gilles Peskine --- tests/include/test/threading_helpers.h | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/include/test/threading_helpers.h b/tests/include/test/threading_helpers.h index 79bc6c0de..dbe2f4c8c 100644 --- a/tests/include/test/threading_helpers.h +++ b/tests/include/test/threading_helpers.h @@ -13,18 +13,31 @@ #ifndef THREADING_HELPERS_H #define THREADING_HELPERS_H -#if defined MBEDTLS_THREADING_C - #include "mbedtls/private_access.h" #include "mbedtls/build_info.h" +#if defined MBEDTLS_THREADING_C + +#include + /* Most fields of publicly available structs are private and are wrapped with * MBEDTLS_PRIVATE macro. This define allows tests to access the private fields * directly (without using the MBEDTLS_PRIVATE wrapper). */ #define MBEDTLS_ALLOW_PRIVATE_ACCESS +/* Error in thread management */ #define MBEDTLS_ERR_THREADING_THREAD_ERROR -0x001F +/* Error in mutex usage (used in Mbed TLS up to 3.6, no longer used + * outside the test framework since TF-PSA-Crypto 1.0). + * + * In Mbed TLS 3.5, this is a redefinition of the macro to the same + * value (down to the exact sequence of tokens and presence/absence of + * whitespace between tokens), which is valid C. + */ +#define MBEDTLS_ERR_THREADING_BAD_INPUT_DATA -0x001C + + #if defined(MBEDTLS_THREADING_PTHREAD) #include #endif /* MBEDTLS_THREADING_PTHREAD */