Initialize driver context in setup functions
In API functions that set up a multipart or interruptible operation, make
sure to initialize the driver-specific part of the context. This is a union,
and initializing the union to `{0}` only guarantees that the first member of
the union is initialized, not necessarily the member used by the driver.
Most compilers do initialize the whole union to all-bits-zero, but some
don't. With compilers that don't, the lack of initialization caused failures
of built-in MAC, interruptible-sign and interruptible-verify. It could also
cause failures for other operations with third-party drivers: we promise
that drivers' setup entry points receive a zero-initialized operation
structure, but this promise was not kept.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index e7f99ad..f043411 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -2400,8 +2400,11 @@
goto exit;
}
- /* Ensure all of the context is zeroized, since PSA_HASH_OPERATION_INIT only
- * directly zeroes the int-sized dummy member of the context union. */
+ /* Make sure the driver-dependent part of the operation is zeroed.
+ * This is a guarantee we make to drivers. Initializing the operation
+ * does not necessarily take care of it, since the context is a
+ * union and initializing a union does not necessarily initialize
+ * all of its members. */
memset(&operation->ctx, 0, sizeof(operation->ctx));
status = psa_driver_wrapper_hash_setup(operation, alg);
@@ -2596,6 +2599,13 @@
return PSA_ERROR_BAD_STATE;
}
+ /* Make sure the driver-dependent part of the operation is zeroed.
+ * This is a guarantee we make to drivers. Initializing the operation
+ * does not necessarily take care of it, since the context is a
+ * union and initializing a union does not necessarily initialize
+ * all of its members. */
+ memset(&target_operation->ctx, 0, sizeof(target_operation->ctx));
+
psa_status_t status = psa_driver_wrapper_hash_clone(source_operation,
target_operation);
if (status != PSA_SUCCESS) {
@@ -2693,6 +2703,13 @@
goto exit;
}
+ /* Make sure the driver-dependent part of the operation is zeroed.
+ * This is a guarantee we make to drivers. Initializing the operation
+ * does not necessarily take care of it, since the context is a
+ * union and initializing a union does not necessarily initialize
+ * all of its members. */
+ memset(&operation->ctx, 0, sizeof(operation->ctx));
+
status = psa_get_and_lock_key_slot_with_policy(
key,
&slot,
@@ -3619,6 +3636,13 @@
return PSA_ERROR_BAD_STATE;
}
+ /* Make sure the driver-dependent part of the operation is zeroed.
+ * This is a guarantee we make to drivers. Initializing the operation
+ * does not necessarily take care of it, since the context is a
+ * union and initializing a union does not necessarily initialize
+ * all of its members. */
+ memset(&operation->ctx, 0, sizeof(operation->ctx));
+
status = psa_sign_verify_check_alg(0, alg);
if (status != PSA_SUCCESS) {
operation->error_occurred = 1;
@@ -3779,6 +3803,13 @@
return PSA_ERROR_BAD_STATE;
}
+ /* Make sure the driver-dependent part of the operation is zeroed.
+ * This is a guarantee we make to drivers. Initializing the operation
+ * does not necessarily take care of it, since the context is a
+ * union and initializing a union does not necessarily initialize
+ * all of its members. */
+ memset(&operation->ctx, 0, sizeof(operation->ctx));
+
status = psa_sign_verify_check_alg(0, alg);
if (status != PSA_SUCCESS) {
operation->error_occurred = 1;
@@ -4446,6 +4477,14 @@
}
operation->default_iv_length = PSA_CIPHER_IV_LENGTH(slot->attr.type, alg);
+
+ /* Make sure the driver-dependent part of the operation is zeroed.
+ * This is a guarantee we make to drivers. Initializing the operation
+ * does not necessarily take care of it, since the context is a
+ * union and initializing a union does not necessarily initialize
+ * all of its members. */
+ memset(&operation->ctx, 0, sizeof(operation->ctx));
+
/* Try doing the operation through a driver before using software fallback. */
if (cipher_operation == MBEDTLS_ENCRYPT) {
status = psa_driver_wrapper_cipher_encrypt_setup(operation,
@@ -5079,6 +5118,13 @@
goto exit;
}
+ /* Make sure the driver-dependent part of the operation is zeroed.
+ * This is a guarantee we make to drivers. Initializing the operation
+ * does not necessarily take care of it, since the context is a
+ * union and initializing a union does not necessarily initialize
+ * all of its members. */
+ memset(&operation->ctx, 0, sizeof(operation->ctx));
+
if (is_encrypt) {
key_usage = PSA_KEY_USAGE_ENCRYPT;
} else {
@@ -8629,7 +8675,11 @@
goto exit;
}
- memset(&operation->data.inputs, 0, sizeof(operation->data.inputs));
+ /* Make sure the variable-purpose part of the operation is zeroed.
+ * Initializing the operation does not necessarily take care of it,
+ * since the context is a union and initializing a union does not
+ * necessarily initialize all of its members. */
+ memset(&operation->data, 0, sizeof(operation->data));
operation->alg = cipher_suite->algorithm;
operation->primitive = PSA_PAKE_PRIMITIVE(cipher_suite->type,