Merge pull request #4154 from chris-jones-arm/test-mutex-usage-2.16

Backport 2.16: test and fix mutex usage

tests/scripts/all.sh

@@ -1324,7 +1324,7 @@ component_test_malloc_0_null () {
     msg "build: malloc(0) returns NULL (ASan+UBSan build)"
     scripts/config.pl full
     scripts/config.pl unset MBEDTLS_MEMORY_BUFFER_ALLOC_C
-    make CC=gcc CFLAGS="'-DMBEDTLS_CONFIG_FILE=\"$PWD/tests/configs/config-wrapper-malloc-0-null.h\"' -O -Werror -Wall -Wextra -fsanitize=address,undefined" LDFLAGS='-fsanitize=address,undefined'
+    make CC=gcc CFLAGS="'-DMBEDTLS_CONFIG_FILE=\"$PWD/tests/configs/config-wrapper-malloc-0-null.h\"' -O $ASAN_CFLAGS" LDFLAGS="$ASAN_CFLAGS"
 
     msg "test: malloc(0) returns NULL (ASan+UBSan build)"
     make test

tests/suites/helpers.function

@@ -46,6 +46,12 @@ typedef UINT32 uint32_t;
 #include <strings.h>
 #endif
 
+#if defined(MBEDTLS_THREADING_C) && defined(MBEDTLS_THREADING_PTHREAD) && \
+    defined(MBEDTLS_TEST_HOOKS)
+#include "mbedtls/threading.h"
+#define MBEDTLS_TEST_MUTEX_USAGE
+#endif
+
 /*
  * Define the two macros
  *
@@ -371,6 +377,9 @@ static struct
     const char *test;
     const char *filename;
     int line_no;
+#if defined(MBEDTLS_TEST_MUTEX_USAGE)
+    const char *mutex_usage_error;
+#endif
 }
 test_info;
 
@@ -777,3 +786,202 @@ int mbedtls_test_hexcmp( uint8_t * a, uint8_t * b, uint32_t a_len, uint32_t b_le
     }
     return ret;
 }
+
+#if defined(MBEDTLS_TEST_MUTEX_USAGE)
+/** Mutex usage verification framework.
+ *
+ * The mutex usage verification code below aims to detect bad usage of
+ * Mbed TLS's mutex abstraction layer at runtime. Note that this is solely
+ * about the use of the mutex itself, not about checking whether the mutex
+ * correctly protects whatever it is supposed to protect.
+ *
+ * The normal usage of a mutex is:
+ * ```
+ * digraph mutex_states {
+ *   "UNINITIALIZED"; // the initial state
+ *   "IDLE";
+ *   "FREED";
+ *   "LOCKED";
+ *   "UNINITIALIZED" -> "IDLE" [label="init"];
+ *   "FREED" -> "IDLE" [label="init"];
+ *   "IDLE" -> "LOCKED" [label="lock"];
+ *   "LOCKED" -> "IDLE" [label="unlock"];
+ *   "IDLE" -> "FREED" [label="free"];
+ * }
+ * ```
+ *
+ * All bad transitions that can be unambiguously detected are reported.
+ * An attempt to use an uninitialized mutex cannot be detected in general
+ * since the memory content may happen to denote a valid state. For the same
+ * reason, a double init cannot be detected.
+ * All-bits-zero is the state of a freed mutex, which is distinct from an
+ * initialized mutex, so attempting to use zero-initialized memory as a mutex
+ * without calling the init function is detected.
+ *
+ * The framework attempts to detect missing calls to init and free by counting
+ * calls to init and free. If there are more calls to init than free, this
+ * means that a mutex is not being freed somewhere, which is a memory leak
+ * on platforms where a mutex consumes resources other than the
+ * mbedtls_threading_mutex_t object itself. If there are more calls to free
+ * than init, this indicates a missing init, which is likely to be detected
+ * by an attempt to lock the mutex as well. A limitation of this framework is
+ * that it cannot detect scenarios where there is exactly the same number of
+ * calls to init and free but the calls don't match. A bug like this is
+ * unlikely to happen uniformly throughout the whole test suite though.
+ *
+ * If an error is detected, this framework will report what happened and the
+ * test case will be marked as failed. Unfortunately, the error report cannot
+ * indicate the exact location of the problematic call. To locate the error,
+ * use a debugger and set a breakpoint on mbedtls_test_mutex_usage_error().
+ */
+enum value_of_mutex_is_valid_field
+{
+    /* Potential values for the is_valid field of mbedtls_threading_mutex_t.
+     * Note that MUTEX_FREED must be 0 and MUTEX_IDLE must be 1 for
+     * compatibility with threading_mutex_init_pthread() and
+     * threading_mutex_free_pthread(). MUTEX_LOCKED could be any nonzero
+     * value. */
+    MUTEX_FREED = 0, //!< Set by threading_mutex_free_pthread
+    MUTEX_IDLE = 1, //!< Set by threading_mutex_init_pthread and by our unlock
+    MUTEX_LOCKED = 2, //!< Set by our lock
+};
+
+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 * );
+} mutex_functions_t;
+static mutex_functions_t mutex_functions;
+
+/** The total number of calls to mbedtls_mutex_init(), minus the total number
+ * of calls to mbedtls_mutex_free().
+ *
+ * Reset to 0 after each test case.
+ */
+static int live_mutexes;
+
+static void mbedtls_test_mutex_usage_error( mbedtls_threading_mutex_t *mutex,
+                                            const char *msg )
+{
+    (void) mutex;
+    if( test_info.mutex_usage_error == NULL )
+        test_info.mutex_usage_error = msg;
+    mbedtls_fprintf( stdout, "[mutex: %s] ", msg );
+    /* Don't mark the test as failed yet. This way, if the test fails later
+     * for a functional reason, the test framework will report the message
+     * and location for this functional reason. If the test passes,
+     * mbedtls_test_mutex_usage_check() will mark it as failed. */
+}
+
+static void mbedtls_test_wrap_mutex_init( mbedtls_threading_mutex_t *mutex )
+{
+    mutex_functions.init( mutex );
+    if( mutex->is_valid )
+        ++live_mutexes;
+}
+
+static void mbedtls_test_wrap_mutex_free( mbedtls_threading_mutex_t *mutex )
+{
+    switch( mutex->is_valid )
+    {
+        case MUTEX_FREED:
+            mbedtls_test_mutex_usage_error( mutex, "free without init or double free" );
+            break;
+        case MUTEX_IDLE:
+            /* Do nothing. The underlying free function will reset is_valid
+             * to 0. */
+            break;
+        case MUTEX_LOCKED:
+            mbedtls_test_mutex_usage_error( mutex, "free without unlock" );
+            break;
+        default:
+            mbedtls_test_mutex_usage_error( mutex, "corrupted state" );
+            break;
+    }
+    if( mutex->is_valid )
+        --live_mutexes;
+    mutex_functions.free( mutex );
+}
+
+static int mbedtls_test_wrap_mutex_lock( mbedtls_threading_mutex_t *mutex )
+{
+    int ret = mutex_functions.lock( mutex );
+    switch( mutex->is_valid )
+    {
+        case MUTEX_FREED:
+            mbedtls_test_mutex_usage_error( mutex, "lock without init" );
+            break;
+        case MUTEX_IDLE:
+            if( ret == 0 )
+                mutex->is_valid = 2;
+            break;
+        case MUTEX_LOCKED:
+            mbedtls_test_mutex_usage_error( mutex, "double lock" );
+            break;
+        default:
+            mbedtls_test_mutex_usage_error( mutex, "corrupted state" );
+            break;
+    }
+    return( ret );
+}
+
+static int mbedtls_test_wrap_mutex_unlock( mbedtls_threading_mutex_t *mutex )
+{
+    int ret = mutex_functions.unlock( mutex );
+    switch( mutex->is_valid )
+    {
+        case MUTEX_FREED:
+            mbedtls_test_mutex_usage_error( mutex, "unlock without init" );
+            break;
+        case MUTEX_IDLE:
+            mbedtls_test_mutex_usage_error( mutex, "unlock without lock" );
+            break;
+        case MUTEX_LOCKED:
+            if( ret == 0 )
+                mutex->is_valid = MUTEX_IDLE;
+            break;
+        default:
+            mbedtls_test_mutex_usage_error( mutex, "corrupted state" );
+            break;
+    }
+    return( ret );
+}
+
+static 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;
+}
+
+static void mbedtls_test_mutex_usage_check( void )
+{
+    if( live_mutexes != 0 )
+    {
+        /* A positive number (more init than free) means that a mutex resource
+         * is leaking (on platforms where a mutex consumes more than the
+         * mbedtls_threading_mutex_t object itself). The rare case of a
+         * negative number means a missing init somewhere. */
+        mbedtls_fprintf( stdout, "[mutex: %d leaked] ", live_mutexes );
+        live_mutexes = 0;
+        if( test_info.mutex_usage_error == NULL )
+            test_info.mutex_usage_error = "missing free";
+    }
+    if( test_info.mutex_usage_error != NULL &&
+        test_info.result != TEST_RESULT_FAILED )
+    {
+        /* Functionally, the test passed. But there was a mutex usage error,
+         * so mark the test as failed after all. */
+        test_fail( "Mutex usage error", __LINE__, __FILE__ );
+    }
+    test_info.mutex_usage_error = NULL;
+}
+
+#endif /* MBEDTLS_TEST_MUTEX_USAGE */

tests/suites/host_test.function

@@ -412,6 +412,10 @@ int execute_tests( int argc , const char ** argv )
     mbedtls_memory_buffer_alloc_init( alloc_buf, sizeof( alloc_buf ) );
 #endif
 
+#if defined(MBEDTLS_TEST_MUTEX_USAGE)
+    mbedtls_test_mutex_usage_init( );
+#endif
+
     /*
      * The C standard doesn't guarantee that all-bits-0 is the representation
      * of a NULL pointer. We do however use that in our code for initializing

tests/suites/main_test.function

@@ -176,6 +176,10 @@ void execute_function_ptr(TestWrapper_t fp, void **params)
 #else
     fp( params );
 #endif
+
+#if defined(MBEDTLS_TEST_MUTEX_USAGE)
+    mbedtls_test_mutex_usage_check( );
+#endif /* MBEDTLS_TEST_MUTEX_USAGE */
 }
 
 /**

tests/suites/test_suite_entropy.data

@@ -1,3 +1,9 @@
+Entropy init-free-free
+entropy_init_free:0
+
+Entropy init-free-init-free
+entropy_init_free:1
+
 Create NV seed_file
 nv_seed_file_create:
 

tests/suites/test_suite_entropy.function

@@ -125,6 +125,28 @@ int read_nv_seed( unsigned char *buf, size_t buf_len )
  * END_DEPENDENCIES
  */
 
+/* BEGIN_CASE */
+void entropy_init_free( int reinit )
+{
+    mbedtls_entropy_context ctx;
+
+    /* Double free is not explicitly documented to work, but it is convenient
+     * to call mbedtls_entropy_free() unconditionally on an error path without
+     * checking whether it has already been called in the success path. */
+
+    mbedtls_entropy_init( &ctx );
+    mbedtls_entropy_free( &ctx );
+
+    if( reinit )
+        mbedtls_entropy_init( &ctx );
+    mbedtls_entropy_free( &ctx );
+
+    /* This test case always succeeds, functionally speaking. A plausible
+     * bug might trigger an invalid pointer dereference or a memory leak. */
+    goto exit;
+}
+/* END_CASE */
+
 /* BEGIN_CASE depends_on:MBEDTLS_ENTROPY_NV_SEED:MBEDTLS_FS_IO */
 void entropy_seed_file( char * path, int ret )
 {
@@ -191,6 +213,9 @@ void entropy_func_len( int len, int ret )
 
     for( j = len; j < sizeof( buf ); j++ )
         TEST_ASSERT( acc[j] == 0 );
+
+exit:
+    mbedtls_entropy_free( &ctx );
 }
 /* END_CASE */
 

tests/suites/test_suite_rsa.data

@@ -1,6 +1,12 @@
 RSA parameter validation
 rsa_invalid_param:
 
+RSA init-free-free
+rsa_init_free:0
+
+RSA init-free-init-free
+rsa_init_free:1
+
 RSA PKCS1 Verify v1.5 CAVS #1
 depends_on:MBEDTLS_SHA1_C:MBEDTLS_PKCS1_V15
 # Good padding but wrong hash

tests/suites/test_suite_rsa.function

@@ -465,6 +465,29 @@ exit:
 }
 /* END_CASE */
 
+/* BEGIN_CASE */
+void rsa_init_free( int reinit )
+{
+    mbedtls_rsa_context ctx;
+
+    /* Double free is not explicitly documented to work, but we rely on it
+     * even inside the library so that you can call mbedtls_rsa_free()
+     * unconditionally on an error path without checking whether it has
+     * already been called in the success path. */
+
+    mbedtls_rsa_init( &ctx, 0, 0 );
+    mbedtls_rsa_free( &ctx );
+
+    if( reinit )
+        mbedtls_rsa_init( &ctx, 0, 0 );
+    mbedtls_rsa_free( &ctx );
+
+    /* This test case always succeeds, functionally speaking. A plausible
+     * bug might trigger an invalid pointer dereference or a memory leak. */
+    goto exit;
+}
+/* END_CASE */
+
 /* BEGIN_CASE */
 void mbedtls_rsa_pkcs1_sign( data_t * message_str, int padding_mode,
                              int digest, int mod, int radix_P, char * input_P,