Tweak the behaviour of copy handling macros

Specifically:
* Move the creation of the pointer to the copied buffer into the
  DECLARE() macro, to solve warnings about potentially skipping
  initialization.
* Reorder the arguments of the FREE() macro - having a different order
  made it confusing, so keep the order the same throughout.

Signed-off-by: David Horstmann <david.horstmann@arm.com>
This commit is contained in:
David Horstmann 2023-12-14 18:12:47 +00:00
parent 853f9f97eb
commit 62a56d966d

View File

@ -112,7 +112,8 @@ mbedtls_psa_drbg_context_t *const mbedtls_psa_random_state =
#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) #if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
/* Declare a local copy of an input buffer. /* Declare a local copy of an input buffer and a variable that will be used
* to store a pointer to the start of the buffer.
* *
* Note: This macro must be called before any operations which may jump to * Note: This macro must be called before any operations which may jump to
* the exit label, so that the local input copy object is safe to be freed. * the exit label, so that the local input copy object is safe to be freed.
@ -120,39 +121,41 @@ mbedtls_psa_drbg_context_t *const mbedtls_psa_random_state =
* Assumptions: * Assumptions:
* - input is the name of a pointer to the buffer to be copied * - input is the name of a pointer to the buffer to be copied
* - The name LOCAL_INPUT_COPY_OF_input is unused in the current scope * - The name LOCAL_INPUT_COPY_OF_input is unused in the current scope
* - input_copy_name is a name that is unused in the current scope
*/ */
#define LOCAL_INPUT_DECLARE(input) \ #define LOCAL_INPUT_DECLARE(input, input_copy_name) \
psa_crypto_local_input_t LOCAL_INPUT_COPY_OF_##input = PSA_CRYPTO_LOCAL_INPUT_INIT; psa_crypto_local_input_t LOCAL_INPUT_COPY_OF_##input = PSA_CRYPTO_LOCAL_INPUT_INIT; \
const uint8_t *input_copy_name = NULL;
/* Allocate a copy of the buffer input and create a pointer with the name /* Allocate a copy of the buffer input and set the pointer input_copy to
* input_copy_name that points to the start of the copy. * point to the start of the copy.
* *
* Assumptions: * Assumptions:
* - psa_status_t status exists * - psa_status_t status exists
* - An exit label is declared * - An exit label is declared
* - input is the name of a pointer to the buffer to be copied * - input is the name of a pointer to the buffer to be copied
* - LOCAL_INPUT_DECLARE(input) has previously been called * - LOCAL_INPUT_DECLARE(input, input_copy) has previously been called
* - input_copy_name is a name that is not used in the current scope
*/ */
#define LOCAL_INPUT_ALLOC(input, length, input_copy_name) \ #define LOCAL_INPUT_ALLOC(input, length, input_copy) \
status = psa_crypto_local_input_alloc(input, length, \ status = psa_crypto_local_input_alloc(input, length, \
&LOCAL_INPUT_COPY_OF_##input); \ &LOCAL_INPUT_COPY_OF_##input); \
if (status != PSA_SUCCESS) { \ if (status != PSA_SUCCESS) { \
goto exit; \ goto exit; \
} \ } \
const uint8_t *input_copy_name = LOCAL_INPUT_COPY_OF_##input.buffer; input_copy = LOCAL_INPUT_COPY_OF_##input.buffer;
/* Free the local input copy allocated previously by LOCAL_INPUT_ALLOC() /* Free the local input copy allocated previously by LOCAL_INPUT_ALLOC()
* *
* Assumptions: * Assumptions:
* - input_copy_name is the name of the input copy created by LOCAL_INPUT_ALLOC() * - input_copy is the name of the input copy pointer set by LOCAL_INPUT_ALLOC()
* - input is the name of the original buffer that was copied * - input is the name of the original buffer that was copied
*/ */
#define LOCAL_INPUT_FREE(input_copy_name, input) \ #define LOCAL_INPUT_FREE(input, input_copy) \
input_copy_name = NULL; \ input_copy = NULL; \
psa_crypto_local_input_free(&LOCAL_INPUT_COPY_OF_##input); psa_crypto_local_input_free(&LOCAL_INPUT_COPY_OF_##input);
/* Declare a local copy of an output buffer. /* Declare a local copy of an output buffer and a variable that will be used
* to store a pointer to the start of the buffer.
* *
* Note: This macro must be called before any operations which may jump to * Note: This macro must be called before any operations which may jump to
* the exit label, so that the local output copy object is safe to be freed. * the exit label, so that the local output copy object is safe to be freed.
@ -160,38 +163,39 @@ mbedtls_psa_drbg_context_t *const mbedtls_psa_random_state =
* Assumptions: * Assumptions:
* - output is the name of a pointer to the buffer to be copied * - output is the name of a pointer to the buffer to be copied
* - The name LOCAL_OUTPUT_COPY_OF_output is unused in the current scope * - The name LOCAL_OUTPUT_COPY_OF_output is unused in the current scope
* - output_copy_name is a name that is unused in the current scope
*/ */
#define LOCAL_OUTPUT_DECLARE(output) \ #define LOCAL_OUTPUT_DECLARE(output, output_copy_name) \
psa_crypto_local_output_t LOCAL_OUTPUT_COPY_OF_##output = PSA_CRYPTO_LOCAL_OUTPUT_INIT; psa_crypto_local_output_t LOCAL_OUTPUT_COPY_OF_##output = PSA_CRYPTO_LOCAL_OUTPUT_INIT; \
uint8_t *output_copy_name = NULL;
/* Allocate a copy of the buffer output and create a pointer with the name /* Allocate a copy of the buffer output and set the pointer output_copy to
* output_copy_name that points to the start of the copy. * point to the start of the copy.
* *
* Assumptions: * Assumptions:
* - psa_status_t status exists * - psa_status_t status exists
* - An exit label is declared * - An exit label is declared
* - output is the name of a pointer to the buffer to be copied * - output is the name of a pointer to the buffer to be copied
* - LOCAL_OUTPUT_DECLARE(output) has previously been called * - LOCAL_OUTPUT_DECLARE(output, output_copy) has previously been called
* - output_copy_name is a name that is not used in the current scope
*/ */
#define LOCAL_OUTPUT_ALLOC(output, length, output_copy_name) \ #define LOCAL_OUTPUT_ALLOC(output, length, output_copy) \
status = psa_crypto_local_output_alloc(output, length, \ status = psa_crypto_local_output_alloc(output, length, \
&LOCAL_OUTPUT_COPY_OF_##output); \ &LOCAL_OUTPUT_COPY_OF_##output); \
if (status != PSA_SUCCESS) { \ if (status != PSA_SUCCESS) { \
goto exit; \ goto exit; \
} \ } \
uint8_t *output_copy_name = LOCAL_OUTPUT_COPY_OF_##output.buffer; output_copy = LOCAL_OUTPUT_COPY_OF_##output.buffer;
/* Free the local input copy allocated previously by LOCAL_INPUT_ALLOC() /* Free the local output copy allocated previously by LOCAL_OUTPUT_ALLOC()
* after first copying back its contents to the original buffer. * after first copying back its contents to the original buffer.
* *
* Assumptions: * Assumptions:
* - psa_status_t status exists * - psa_status_t status exists
* - input_copy_name is the name of the input copy created by LOCAL_INPUT_ALLOC() * - output_copy is the name of the output copy pointer set by LOCAL_OUTPUT_ALLOC()
* - input is the name of the original buffer that was copied * - output is the name of the original buffer that was copied
*/ */
#define LOCAL_OUTPUT_FREE(output_copy_name, output) \ #define LOCAL_OUTPUT_FREE(output, output_copy) \
output_copy_name = NULL; \ output_copy = NULL; \
do { \ do { \
psa_status_t local_output_status; \ psa_status_t local_output_status; \
local_output_status = psa_crypto_local_output_free(&LOCAL_OUTPUT_COPY_OF_##output); \ local_output_status = psa_crypto_local_output_free(&LOCAL_OUTPUT_COPY_OF_##output); \
@ -203,17 +207,18 @@ mbedtls_psa_drbg_context_t *const mbedtls_psa_random_state =
} \ } \
} while (0) } while (0)
#else /* MBEDTLS_PSA_COPY_CALLER_BUFFERS */ #else /* MBEDTLS_PSA_COPY_CALLER_BUFFERS */
#define LOCAL_INPUT_DECLARE(input) #define LOCAL_INPUT_DECLARE(input, input_copy_name) \
#define LOCAL_INPUT_ALLOC(input, length, input_copy_name) \ const uint8_t *input_copy_name = NULL;
const uint8_t *input_copy_name = input; #define LOCAL_INPUT_ALLOC(input, length, input_copy) \
#define LOCAL_INPUT_FREE(input_copy_name, input) \ input_copy = input;
input_copy_name = NULL; #define LOCAL_INPUT_FREE(input, input_copy) \
#define LOCAL_OUTPUT_DECLARE(output) input_copy = NULL;
#define LOCAL_OUTPUT_ALLOC(output, length, output_copy_name) \ #define LOCAL_OUTPUT_DECLARE(output, output_copy_name) \
uint8_t *output_copy_name = output; uint8_t *output_copy_name = NULL;
#define LOCAL_OUTPUT_FREE(output_copy_name, output) \ #define LOCAL_OUTPUT_ALLOC(output, length, output_copy) \
output_copy_name = NULL; output_copy = output;
#define LOCAL_OUTPUT_FREE(output, output_copy) \
output_copy = NULL;
#endif /* MBEDTLS_PSA_COPY_CALLER_BUFFERS */ #endif /* MBEDTLS_PSA_COPY_CALLER_BUFFERS */
@ -4449,9 +4454,10 @@ psa_status_t psa_cipher_encrypt(mbedtls_svc_key_id_t key,
size_t default_iv_length = 0; size_t default_iv_length = 0;
psa_key_attributes_t attributes; psa_key_attributes_t attributes;
LOCAL_INPUT_DECLARE(input_external); LOCAL_INPUT_DECLARE(input_external, input);
LOCAL_OUTPUT_DECLARE(output_external, output);
LOCAL_INPUT_ALLOC(input_external, input_length, input); LOCAL_INPUT_ALLOC(input_external, input_length, input);
LOCAL_OUTPUT_DECLARE(output_external);
LOCAL_OUTPUT_ALLOC(output_external, output_size, output); LOCAL_OUTPUT_ALLOC(output_external, output_size, output);
if (!PSA_ALG_IS_CIPHER(alg)) { if (!PSA_ALG_IS_CIPHER(alg)) {
@ -4509,8 +4515,8 @@ exit:
*output_length = 0; *output_length = 0;
} }
LOCAL_INPUT_FREE(input, input_external); LOCAL_INPUT_FREE(input_external, input);
LOCAL_OUTPUT_FREE(output, output_external); LOCAL_OUTPUT_FREE(output_external, output);
return status; return status;
} }