From 321c7e9ed9559212b11011029cf01c43a2b65419 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 28 Sep 2023 18:07:06 +0100 Subject: [PATCH 1/4] Fix error handling in psa_driver_wrapper_xxx_hash_get_num_ops Signed-off-by: Dave Rodgman --- library/psa_crypto.c | 9 +++++--- .../psa_crypto_driver_wrappers.h.jinja | 22 +++++++++++-------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 1faf1dd6c..8e339ea79 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3470,7 +3470,9 @@ psa_status_t psa_sign_hash_complete( signature_length); /* Update ops count with work done. */ - operation->num_ops = psa_driver_wrapper_sign_hash_get_num_ops(operation); + if (status == PSA_SUCCESS) { + status = psa_driver_wrapper_sign_hash_get_num_ops(operation, &operation->num_ops); + } exit: @@ -3601,8 +3603,9 @@ psa_status_t psa_verify_hash_complete( status = psa_driver_wrapper_verify_hash_complete(operation); /* Update ops count with work done. */ - operation->num_ops = psa_driver_wrapper_verify_hash_get_num_ops( - operation); + if (status == PSA_SUCCESS) { + status = psa_driver_wrapper_verify_hash_get_num_ops(operation, &operation->num_ops); + } exit: diff --git a/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.h.jinja b/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.h.jinja index 3d116b396..4032d0e99 100644 --- a/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.h.jinja +++ b/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.h.jinja @@ -472,17 +472,19 @@ static inline psa_status_t psa_driver_wrapper_verify_hash( } } -static inline uint32_t psa_driver_wrapper_sign_hash_get_num_ops( - psa_sign_hash_interruptible_operation_t *operation ) +static inline int psa_driver_wrapper_sign_hash_get_num_ops( + psa_sign_hash_interruptible_operation_t *operation, uint32_t *num_ops ) { switch( operation->id ) { /* If uninitialised, return 0, as no work can have been done. */ case 0: - return 0; + *num_ops = 0; + return PSA_SUCCESS; case PSA_CRYPTO_MBED_TLS_DRIVER_ID: - return(mbedtls_psa_sign_hash_get_num_ops(&operation->ctx.mbedtls_ctx)); + *num_ops = mbedtls_psa_sign_hash_get_num_ops(&operation->ctx.mbedtls_ctx); + return PSA_SUCCESS; #if defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) #if defined(PSA_CRYPTO_DRIVER_TEST) @@ -495,17 +497,19 @@ static inline uint32_t psa_driver_wrapper_sign_hash_get_num_ops( return( PSA_ERROR_INVALID_ARGUMENT ); } -static inline uint32_t psa_driver_wrapper_verify_hash_get_num_ops( - psa_verify_hash_interruptible_operation_t *operation ) +static inline int psa_driver_wrapper_verify_hash_get_num_ops( + psa_verify_hash_interruptible_operation_t *operation, uint32_t *num_ops ) { switch( operation->id ) { /* If uninitialised, return 0, as no work can have been done. */ case 0: - return 0; + *num_ops = 0; + return PSA_SUCCESS; case PSA_CRYPTO_MBED_TLS_DRIVER_ID: - return (mbedtls_psa_verify_hash_get_num_ops(&operation->ctx.mbedtls_ctx)); + *num_ops = mbedtls_psa_verify_hash_get_num_ops(&operation->ctx.mbedtls_ctx); + return PSA_SUCCESS; #if defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) #if defined(PSA_CRYPTO_DRIVER_TEST) @@ -516,7 +520,7 @@ static inline uint32_t psa_driver_wrapper_verify_hash_get_num_ops( } - return( PSA_ERROR_INVALID_ARGUMENT ); + return ( PSA_ERROR_INVALID_ARGUMENT ); } static inline psa_status_t psa_driver_wrapper_sign_hash_start( From 2bc38a6dfe63fd2acb47b0ac390dfddd96c17d7e Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 28 Sep 2023 18:15:58 +0100 Subject: [PATCH 2/4] Fix return type Signed-off-by: Dave Rodgman --- .../driver_templates/psa_crypto_driver_wrappers.h.jinja | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.h.jinja b/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.h.jinja index 4032d0e99..282dd7bf7 100644 --- a/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.h.jinja +++ b/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.h.jinja @@ -472,7 +472,7 @@ static inline psa_status_t psa_driver_wrapper_verify_hash( } } -static inline int psa_driver_wrapper_sign_hash_get_num_ops( +static inline psa_status_t psa_driver_wrapper_sign_hash_get_num_ops( psa_sign_hash_interruptible_operation_t *operation, uint32_t *num_ops ) { switch( operation->id ) @@ -497,7 +497,7 @@ static inline int psa_driver_wrapper_sign_hash_get_num_ops( return( PSA_ERROR_INVALID_ARGUMENT ); } -static inline int psa_driver_wrapper_verify_hash_get_num_ops( +static inline psa_status_t psa_driver_wrapper_verify_hash_get_num_ops( psa_verify_hash_interruptible_operation_t *operation, uint32_t *num_ops ) { switch( operation->id ) From fe43d12f60f369c975f7dd8b2ac6d06d83b900ba Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 28 Sep 2023 18:46:11 +0100 Subject: [PATCH 3/4] Always call get_num_ops Signed-off-by: Dave Rodgman --- library/psa_crypto.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 8e339ea79..0bb9e8e65 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3448,6 +3448,7 @@ psa_status_t psa_sign_hash_complete( size_t *signature_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + psa_status_t numops_status = PSA_ERROR_CORRUPTION_DETECTED; *signature_length = 0; @@ -3470,8 +3471,9 @@ psa_status_t psa_sign_hash_complete( signature_length); /* Update ops count with work done. */ + numops_status = psa_driver_wrapper_sign_hash_get_num_ops(operation, &operation->num_ops); if (status == PSA_SUCCESS) { - status = psa_driver_wrapper_sign_hash_get_num_ops(operation, &operation->num_ops); + status = numops_status; } exit: @@ -3592,6 +3594,7 @@ psa_status_t psa_verify_hash_complete( psa_verify_hash_interruptible_operation_t *operation) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + psa_status_t numops_status = PSA_ERROR_CORRUPTION_DETECTED; /* Check that start has been called first, and that operation has not * previously errored. */ @@ -3603,8 +3606,9 @@ psa_status_t psa_verify_hash_complete( status = psa_driver_wrapper_verify_hash_complete(operation); /* Update ops count with work done. */ + numops_status = psa_driver_wrapper_verify_hash_get_num_ops(operation, &operation->num_ops); if (status == PSA_SUCCESS) { - status = psa_driver_wrapper_verify_hash_get_num_ops(operation, &operation->num_ops); + status = numops_status; } exit: From 3572bde9c9dd9a631b09b5bfdb05d09dfb2d129a Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 28 Sep 2023 19:33:15 +0100 Subject: [PATCH 4/4] Assume get_num_ops cannot fail Signed-off-by: Dave Rodgman --- library/psa_crypto.c | 13 +++------- .../psa_crypto_driver_wrappers.h.jinja | 26 +++++++++---------- 2 files changed, 15 insertions(+), 24 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 0bb9e8e65..1faf1dd6c 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3448,7 +3448,6 @@ psa_status_t psa_sign_hash_complete( size_t *signature_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; - psa_status_t numops_status = PSA_ERROR_CORRUPTION_DETECTED; *signature_length = 0; @@ -3471,10 +3470,7 @@ psa_status_t psa_sign_hash_complete( signature_length); /* Update ops count with work done. */ - numops_status = psa_driver_wrapper_sign_hash_get_num_ops(operation, &operation->num_ops); - if (status == PSA_SUCCESS) { - status = numops_status; - } + operation->num_ops = psa_driver_wrapper_sign_hash_get_num_ops(operation); exit: @@ -3594,7 +3590,6 @@ psa_status_t psa_verify_hash_complete( psa_verify_hash_interruptible_operation_t *operation) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; - psa_status_t numops_status = PSA_ERROR_CORRUPTION_DETECTED; /* Check that start has been called first, and that operation has not * previously errored. */ @@ -3606,10 +3601,8 @@ psa_status_t psa_verify_hash_complete( status = psa_driver_wrapper_verify_hash_complete(operation); /* Update ops count with work done. */ - numops_status = psa_driver_wrapper_verify_hash_get_num_ops(operation, &operation->num_ops); - if (status == PSA_SUCCESS) { - status = numops_status; - } + operation->num_ops = psa_driver_wrapper_verify_hash_get_num_ops( + operation); exit: diff --git a/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.h.jinja b/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.h.jinja index 282dd7bf7..ded5c041a 100644 --- a/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.h.jinja +++ b/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.h.jinja @@ -472,19 +472,17 @@ static inline psa_status_t psa_driver_wrapper_verify_hash( } } -static inline psa_status_t psa_driver_wrapper_sign_hash_get_num_ops( - psa_sign_hash_interruptible_operation_t *operation, uint32_t *num_ops ) +static inline uint32_t psa_driver_wrapper_sign_hash_get_num_ops( + psa_sign_hash_interruptible_operation_t *operation ) { switch( operation->id ) { /* If uninitialised, return 0, as no work can have been done. */ case 0: - *num_ops = 0; - return PSA_SUCCESS; + return 0; case PSA_CRYPTO_MBED_TLS_DRIVER_ID: - *num_ops = mbedtls_psa_sign_hash_get_num_ops(&operation->ctx.mbedtls_ctx); - return PSA_SUCCESS; + return(mbedtls_psa_sign_hash_get_num_ops(&operation->ctx.mbedtls_ctx)); #if defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) #if defined(PSA_CRYPTO_DRIVER_TEST) @@ -494,22 +492,21 @@ static inline psa_status_t psa_driver_wrapper_sign_hash_get_num_ops( #endif /* PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT */ } - return( PSA_ERROR_INVALID_ARGUMENT ); + /* Can't happen (see discussion in #8271) */ + return 0; } -static inline psa_status_t psa_driver_wrapper_verify_hash_get_num_ops( - psa_verify_hash_interruptible_operation_t *operation, uint32_t *num_ops ) +static inline uint32_t psa_driver_wrapper_verify_hash_get_num_ops( + psa_verify_hash_interruptible_operation_t *operation ) { switch( operation->id ) { /* If uninitialised, return 0, as no work can have been done. */ case 0: - *num_ops = 0; - return PSA_SUCCESS; + return 0; case PSA_CRYPTO_MBED_TLS_DRIVER_ID: - *num_ops = mbedtls_psa_verify_hash_get_num_ops(&operation->ctx.mbedtls_ctx); - return PSA_SUCCESS; + return (mbedtls_psa_verify_hash_get_num_ops(&operation->ctx.mbedtls_ctx)); #if defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) #if defined(PSA_CRYPTO_DRIVER_TEST) @@ -520,7 +517,8 @@ static inline psa_status_t psa_driver_wrapper_verify_hash_get_num_ops( } - return ( PSA_ERROR_INVALID_ARGUMENT ); + /* Can't happen (see discussion in #8271) */ + return 0; } static inline psa_status_t psa_driver_wrapper_sign_hash_start(