From 3d574da6fc0e7ddbb61fad2717d930586ed58bea Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 31 Jul 2023 16:54:00 +0100 Subject: [PATCH] Revert to not enabling asm under Memsan Signed-off-by: Dave Rodgman --- library/constant_time_impl.h | 24 +++++++++--------------- tests/include/test/constant_flow.h | 25 ------------------------- 2 files changed, 9 insertions(+), 40 deletions(-) diff --git a/library/constant_time_impl.h b/library/constant_time_impl.h index b2ef73f700..8570928070 100644 --- a/library/constant_time_impl.h +++ b/library/constant_time_impl.h @@ -37,11 +37,18 @@ #include "mbedtls/bignum.h" #endif -#include "../tests/include/test/constant_flow.h" +/* Disable asm under Memsan because it confuses Memsan and generates false errors */ +#if defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) +#define MBEDTLS_CT_NO_ASM +#elif defined(__has_feature) +#if __has_feature(memory_sanitizer) +#define MBEDTLS_CT_NO_ASM +#endif +#endif /* armcc5 --gnu defines __GNUC__ but doesn't support GNU's extended asm */ #if defined(MBEDTLS_HAVE_ASM) && defined(__GNUC__) && (!defined(__ARMCC_VERSION) || \ - __ARMCC_VERSION >= 6000000) + __ARMCC_VERSION >= 6000000) && !defined(MBEDTLS_CT_NO_ASM) #define MBEDTLS_CT_ASM #if (defined(__arm__) || defined(__thumb__) || defined(__thumb2__)) #define MBEDTLS_CT_ARM_ASM @@ -84,20 +91,7 @@ extern volatile mbedtls_ct_uint_t mbedtls_ct_zero; static inline mbedtls_ct_uint_t mbedtls_ct_compiler_opaque(mbedtls_ct_uint_t x) { #if defined(MBEDTLS_CT_ASM) - /* Prevent false positives from Memsan - otherwise it will report the asm as - * accessing secret data. */ - TEST_CF_SAVE_SECRET(x); - asm volatile ("" : [x] "+r" (x) :); - - /* Mark the return value as secret (if it was previously marked secret). - * This is needed so that code of the form: - * - * if (mbedtls_ct_compiler_opaque(secret)) { ... } - * - * will fail const-flow tests. - */ - TEST_CF_RESTORE_SECRET(x); return x; #else return x ^ mbedtls_ct_zero; diff --git a/tests/include/test/constant_flow.h b/tests/include/test/constant_flow.h index ff464e617c..f3d676e285 100644 --- a/tests/include/test/constant_flow.h +++ b/tests/include/test/constant_flow.h @@ -32,27 +32,14 @@ * #define TEST_CF_SECRET(ptr, size) * #define TEST_CF_PUBLIC(ptr, size) * - * and - * - * #define TEST_CF_SAVE_SECRET(variable) - * #define TEST_CF_RESTORE_SECRET(variable) - * * that can be used in tests to mark a memory area as secret (no branch or * memory access should depend on it) or public (default, only needs to be * marked explicitly when it was derived from secret data). * - * The SAVE/RESTORE forms mark a variable as public, and subsequently restore its - * previous secret/not-secret state. This is used where library code is generating - * false positives and needs to temporarily disable Memsan checks for a particular - * variable, and then restore it's original state afterwards so it doesn't interfere - * with other checks. - * * Arguments: * - ptr: a pointer to the memory area to be marked * - size: the size in bytes of the memory area * - * - variable: a variable name - * * Implementation: * The basic idea is that of ctgrind : we can * re-use tools that were designed for checking use of uninitialized memory. @@ -76,12 +63,6 @@ #define TEST_CF_PUBLIC __msan_unpoison // void __msan_unpoison(const volatile void *a, size_t size); -#define TEST_CF_SAVE_SECRET(_x) \ - int _test_cf_is_public_ ## _x = __msan_test_shadow(&(_x), sizeof(_x)) == -1; \ - TEST_CF_PUBLIC(&(_x), sizeof(_x)); -#define TEST_CF_RESTORE_SECRET(_x) \ - if (!_test_cf_is_public_ ## _x) TEST_CF_SECRET(&(_x), sizeof(_x)); - #elif defined(MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND) #include @@ -90,18 +71,12 @@ #define TEST_CF_PUBLIC VALGRIND_MAKE_MEM_DEFINED // VALGRIND_MAKE_MEM_DEFINED(_qzz_addr, _qzz_len) -#define TEST_CF_SAVE_SECRET(_x) -#define TEST_CF_RESTORE_SECRET(_x) - #else /* MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN || MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND */ #define TEST_CF_SECRET(ptr, size) #define TEST_CF_PUBLIC(ptr, size) -#define TEST_CF_SAVE_SECRET(_x) -#define TEST_CF_RESTORE_SECRET(_x) - #endif /* MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN || MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND */