Change the use of setjmp/longjmp in parameter failure callback

Change the use of setjmp and longjmp in signalling parameter validation failures
when using the MBEDTLS_CHECK_PARAMS config.h option. This change allows
all calls which might result in a call to the parameter validation failure
handler to always be caught, even without use of the new macros, by placing a
setjmp() in the outer function which calls the test function, which the handler
can jump to.

This has several benefits:
    * it allows us to remove the clang compiler warning (-Wclobbered) caused
      by local auto variables being in the same function as the call to setjmp.
    * removes the need to wrap all function calls in the test functions with the
      TEST_ASSERT() macro. Now all parameter validation function calls should be
      caught.
This commit is contained in:
Simon Butcher 2018-12-09 22:09:59 +00:00 committed by Manuel Pégourié-Gonnard
parent 747f5fe3e2
commit 6542f6c597
3 changed files with 97 additions and 101 deletions

View file

@ -27,10 +27,6 @@
#include <setjmp.h> #include <setjmp.h>
#define MBEDTLS_PARAM_FAILED(x) mbedtls_param_failed( #x ) #define MBEDTLS_PARAM_FAILED(x) mbedtls_param_failed( #x )
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic ignored "-Wno-uninitialized"
#endif
#endif /* MBEDTLS_CHECK_PARAMS */ #endif /* MBEDTLS_CHECK_PARAMS */
#ifdef _MSC_VER #ifdef _MSC_VER
@ -75,12 +71,19 @@ typedef struct data_tag
#define DISPATCH_UNSUPPORTED_SUITE -5 /* Test suite not supported by the #define DISPATCH_UNSUPPORTED_SUITE -5 /* Test suite not supported by the
build */ build */
typedef enum
{
PARAMFAIL_TESTSTATE_IDLE = 0, /* No parameter failure call test */
PARAMFAIL_TESTSTATE_PENDING, /* Test call to the parameter failure
* is pending */
PARAMFAIL_TESTSTATE_CALLED /* The test call to the parameter
* failure function has been made */
} paramfail_test_state_t;
/*----------------------------------------------------------------------------*/ /*----------------------------------------------------------------------------*/
/* Macros */ /* Macros */
#if defined(MBEDTLS_CHECK_PARAMS)
/** /**
* \brief This macro tests the expression passed to it as a test step or * \brief This macro tests the expression passed to it as a test step or
* individual test in a test case. * individual test in a test case.
@ -96,53 +99,17 @@ typedef struct data_tag
* *
* \param TEST The test expression to be tested. * \param TEST The test expression to be tested.
*/ */
#define TEST_ASSERT( TEST ) \
do { \ #define TEST_ASSERT( TEST ) \
if ( setjmp( param_fail_jmp ) == 0 ) \ do { \
{ \ if( ! (TEST) ) \
if( ! (TEST) ) \ { \
{ \ test_fail( #TEST, __LINE__, __FILE__ ); \
test_fail( #TEST, __LINE__, __FILE__ ); \ goto exit; \
goto exit; \ } \
} \
} \
else \
{ \
test_fail( #TEST, __LINE__, __FILE__ ); \
goto exit; \
} \
memset( param_fail_jmp, 0, sizeof(jmp_buf) ); \
} while( 0 )
/**
* \brief This macro tests and individual function call as a test step or
* individual test in a test case.
*
* It does not require a library function to return a value, and cannot
tets a return error code that can be tested.
*
* When MBEDTLS_CHECK_PARAMS is enabled, calls to the parameter failure
* callback, MBEDTLS_PARAM_FAIL, will be assumed to be a test failure.
*
* This macro is not suitable for negative parameter validation tests
* as it assumes the test step will not create an error.
*
* \param TEST The test statement to be executed.
*/
#define TEST_FN( TEST ) \
do { \
if ( setjmp( param_fail_jmp ) == 0 ) \
{ \
TEST; \
} \
else \
{ \
test_fail( #TEST, __LINE__, __FILE__ ); \
goto exit; \
} \
memset( param_fail_jmp, 0, sizeof(jmp_buf) ); \
} while( 0 ) } while( 0 )
#if defined(MBEDTLS_CHECK_PARAMS)
/** /**
* \brief This macro tests the statement passed to it as a test step or * \brief This macro tests the statement passed to it as a test step or
* individual test in a test case. The macro assumes the test will fail * individual test in a test case. The macro assumes the test will fail
@ -163,18 +130,16 @@ typedef struct data_tag
* *
* \param TEST The test expression to be tested. * \param TEST The test expression to be tested.
*/ */
#define TEST_INVALID_PARAM_RET( PARAM_ERR_VALUE, TEST ) \ #define TEST_INVALID_PARAM_RET( PARAM_ERR_VALUE, TEST ) \
do { \ do { \
if ( setjmp( param_fail_jmp ) == 0 ) \ test_info.paramfail_test_state = PARAMFAIL_TESTSTATE_PENDING; \
{ \ if( (TEST) != (PARAM_ERR_VALUE) && \
if( (TEST) != PARAM_ERR_VALUE) \ test_info.paramfail_test_state != PARAMFAIL_TESTSTATE_CALLED ) \
{ \ { \
test_fail( #TEST, __LINE__, __FILE__ ); \ test_fail( #TEST, __LINE__, __FILE__ ); \
goto exit; \ goto exit; \
} \ } \
} \ } while( 0 )
memset( param_fail_jmp, 0, sizeof(jmp_buf) ); \
} while( 0 )
/** /**
* \brief This macro tests the statement passed to it as a test step or * \brief This macro tests the statement passed to it as a test step or
@ -196,33 +161,20 @@ typedef struct data_tag
* *
* \param TEST The test expression to be tested. * \param TEST The test expression to be tested.
*/ */
#define TEST_INVALID_PARAM( TEST ) \ #define TEST_INVALID_PARAM( TEST ) \
do { \ do { \
if ( setjmp( param_fail_jmp ) == 0 ) \ memcpy(jmp_tmp, param_fail_jmp, sizeof(jmp_buf)); \
{ \ if ( setjmp( param_fail_jmp ) == 0 ) \
TEST; \ { \
test_fail( #TEST, __LINE__, __FILE__ ); \ TEST; \
goto exit; \ test_fail( #TEST, __LINE__, __FILE__ ); \
} \ goto exit; \
memset( param_fail_jmp, 0, sizeof(jmp_buf) ); \ } \
memcpy(param_fail_jmp, jmp_tmp, sizeof(jmp_buf)); \
} while( 0 ) } while( 0 )
#else #else
#define TEST_ASSERT( TEST ) \
do { \
if( ! (TEST) ) \
{ \
test_fail( #TEST, __LINE__, __FILE__ ); \
goto exit; \
} \
} while( 0 )
#define TEST_FN( TEST ) \
do { \
TEST; \
} while( 0 )
#define TEST_INVALID_PARAM_RET( PARAM_ERR_VALUE, TEST ) \ #define TEST_INVALID_PARAM_RET( PARAM_ERR_VALUE, TEST ) \
do { \ do { \
if( (TEST) != (PARAM_ERR_VALUE) ) \ if( (TEST) != (PARAM_ERR_VALUE) ) \
@ -273,9 +225,9 @@ typedef struct data_tag
/*----------------------------------------------------------------------------*/ /*----------------------------------------------------------------------------*/
/* Global variables */ /* Global variables */
static struct static struct
{ {
paramfail_test_state_t paramfail_test_state;
int failed; int failed;
const char *test; const char *test;
const char *filename; const char *filename;
@ -289,6 +241,7 @@ mbedtls_platform_context platform_ctx;
#if defined(MBEDTLS_CHECK_PARAMS) #if defined(MBEDTLS_CHECK_PARAMS)
jmp_buf param_fail_jmp; jmp_buf param_fail_jmp;
jmp_buf jmp_tmp;
#endif #endif
/*----------------------------------------------------------------------------*/ /*----------------------------------------------------------------------------*/
@ -308,6 +261,15 @@ jmp_buf param_fail_jmp;
/*----------------------------------------------------------------------------*/ /*----------------------------------------------------------------------------*/
/* Helper Functions */ /* Helper Functions */
static void test_fail( const char *test, int line_no, const char* filename )
{
test_info.failed = 1;
test_info.test = test;
test_info.line_no = line_no;
test_info.filename = filename;
}
static int platform_setup() static int platform_setup()
{ {
int ret = 0; int ret = 0;
@ -327,11 +289,22 @@ static void platform_teardown()
#if defined(MBEDTLS_CHECK_PARAMS) #if defined(MBEDTLS_CHECK_PARAMS)
void mbedtls_param_failed( char* failure_condition, char* file, int line ) void mbedtls_param_failed( char* failure_condition, char* file, int line )
{ {
(void)failure_condition; /* If we are testing the callback function... */
(void)file; if ( test_info.paramfail_test_state == PARAMFAIL_TESTSTATE_PENDING )
(void)line; {
test_info.paramfail_test_state = PARAMFAIL_TESTSTATE_CALLED;
}
else
{
/* ...else we treat this as an error */
longjmp( param_fail_jmp, 1 ); /* Record the location of the failure, but not as a failure yet, in case
* it was part of the test */
test_fail( failure_condition, line, file );
test_info.failed = 0;
longjmp( param_fail_jmp, 1 );
}
} }
#endif #endif
@ -623,14 +596,6 @@ static int rnd_pseudo_rand( void *rng_state, unsigned char *output, size_t len )
return( 0 ); return( 0 );
} }
static void test_fail( const char *test, int line_no, const char* filename )
{
test_info.failed = 1;
test_info.test = test;
test_info.line_no = line_no;
test_info.filename = filename;
}
int hexcmp( uint8_t * a, uint8_t * b, uint32_t a_len, uint32_t b_len ) int hexcmp( uint8_t * a, uint8_t * b, uint32_t a_len, uint32_t b_len )
{ {
int ret = 0; int ret = 0;

View file

@ -546,6 +546,7 @@ int execute_tests( int argc , const char ** argv )
if( unmet_dep_count == 0 ) if( unmet_dep_count == 0 )
{ {
test_info.failed = 0; test_info.failed = 0;
test_info.paramfail_test_state = PARAMFAIL_TESTSTATE_IDLE;
#if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) #if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__))
/* Suppress all output from the library unless we're verbose /* Suppress all output from the library unless we're verbose

View file

@ -134,9 +134,39 @@ $dispatch_code
#line $line_no "suites/main_test.function" #line $line_no "suites/main_test.function"
}; };
/**
* \brief Execute the test function.
*
* This is a wrapper function around the test function execution
* to allow the setjmp() call used to catch any calls to the
* parameter failure callback, to be used. Calls to setjmp()
* can invalidate the state of any local auto variables.
*
* \param fp Function pointer to the test function
* \param params Parameters to pass
*
*/
void execute_function_ptr(TestWrapper_t fp, void **params)
{
#if defined(MBEDTLS_CHECK_PARAMS)
if ( setjmp( param_fail_jmp ) == 0 )
{
fp( params );
}
else
{
/* Unexpected parameter validation error */
test_info.failed = 1;
}
memset( param_fail_jmp, 0, sizeof(jmp_buf) );
#else
fp( params );
#endif
}
/** /**
* \brief Dispatches test functions based on function index. * \brief Dispatches test functions based on function index.
* *
* \param exp_id Test function index. * \param exp_id Test function index.
* *
@ -153,7 +183,7 @@ int dispatch_test( int func_idx, void ** params )
{ {
fp = test_funcs[func_idx]; fp = test_funcs[func_idx];
if ( fp ) if ( fp )
fp( params ); execute_function_ptr(fp, params);
else else
ret = DISPATCH_UNSUPPORTED_SUITE; ret = DISPATCH_UNSUPPORTED_SUITE;
} }