commit c9774411d4b90d32b6f0821aca4d53dec461f8c4
author Paul Elliott <paul.elliott@arm.com> Mon Feb 06 15:14:07 2023 +0000
committer Paul Elliott <paul.elliott@arm.com> Wed Feb 15 23:34:29 2023 +0000
tree 746d79f46e099e3df14afd7565415d8b02e381b1
parent f9c91a7fb5766cdb5b85a6dc3292c155e8309c26

Ensure that operation is put into error state if error occurs

If an error occurs, calling any function on the same operation should return
PSA_ERROR_BAD_STATE, and we were not honouring that for all errors. Add extra
failure tests to try and ratify this.

Signed-off-by: Paul Elliott <paul.elliott@arm.com>

library/psa_crypto.c

diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index a3bc806..882cb96 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -3183,14 +3183,15 @@
     psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED;
     psa_key_slot_t *slot;
 
-    /* Check that start has not been previously called. */
-    if (operation->id != 0) {
+    /* Check that start has not been previously called, or operation has not
+     * previously errored. */
+    if (operation->id != 0 || operation->error_occurred) {
         return PSA_ERROR_BAD_STATE;
     }
 
-
     status = psa_sign_verify_check_alg(0, alg);
     if (status != PSA_SUCCESS) {
+        operation->error_occurred = 1;
         return status;
     }
 
@@ -3221,13 +3222,17 @@
 exit:
 
     if (status != PSA_SUCCESS) {
+        operation->error_occurred = 1;
         psa_sign_hash_abort_internal(operation);
     }
 
     unlock_status = psa_unlock_key_slot(slot);
 
-    return (status == PSA_SUCCESS) ? unlock_status : status;
+    if (unlock_status != PSA_SUCCESS) {
+        operation->error_occurred = 1;
+    }
 
+    return (status == PSA_SUCCESS) ? unlock_status : status;
 }
 
 
@@ -3240,8 +3245,9 @@
 
     *signature_length = 0;
 
-    /* Check that start has been called first. */
-    if (operation->id == 0) {
+    /* Check that start has been called first, and that operation has not
+     * previously errored. */
+    if (operation->id == 0 || operation->error_occurred) {
         status = PSA_ERROR_BAD_STATE;
         goto exit;
     }
@@ -3276,6 +3282,10 @@
         /* If signature_size is 0 then we have nothing to do. We must not
          * call memset because signature may be NULL in this case.*/
 
+        if (status != PSA_SUCCESS) {
+            operation->error_occurred = 1;
+        }
+
         psa_sign_hash_abort_internal(operation);
     }
 
@@ -3293,6 +3303,9 @@
      * the operation fails or succeeds, only on manual abort. */
     operation->num_ops = 0;
 
+    /* Likewise, failure state. */
+    operation->error_occurred = 0;
+
     return status;
 }
 
@@ -3325,13 +3338,15 @@
     psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED;
     psa_key_slot_t *slot;
 
-    /* Check that start has not been previously called. */
-    if (operation->id != 0) {
+    /* Check that start has not been previously called, or operation has not
+     * previously errored. */
+    if (operation->id != 0 || operation->error_occurred) {
         return PSA_ERROR_BAD_STATE;
     }
 
     status = psa_sign_verify_check_alg(0, alg);
     if (status != PSA_SUCCESS) {
+        operation->error_occurred = 1;
         return status;
     }
 
@@ -3340,6 +3355,7 @@
                                                    alg);
 
     if (status != PSA_SUCCESS) {
+        operation->error_occurred = 1;
         return status;
     }
 
@@ -3357,14 +3373,17 @@
                                                   signature, signature_length);
 
     if (status != PSA_SUCCESS) {
+        operation->error_occurred = 1;
         psa_verify_hash_abort_internal(operation);
     }
 
     unlock_status = psa_unlock_key_slot(slot);
 
-    return (status == PSA_SUCCESS) ? unlock_status : status;
+    if (unlock_status != PSA_SUCCESS) {
+        operation->error_occurred = 1;
+    }
 
-    return status;
+    return (status == PSA_SUCCESS) ? unlock_status : status;
 }
 
 psa_status_t psa_verify_hash_complete(
@@ -3372,8 +3391,9 @@
 {
     psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
 
-    /* Check that start has been called first. */
-    if (operation->id == 0) {
+    /* Check that start has been called first, and that operation has not
+     * previously errored. */
+    if (operation->id == 0 || operation->error_occurred) {
         status = PSA_ERROR_BAD_STATE;
         goto exit;
     }
@@ -3387,6 +3407,10 @@
         operation);
 
     if (status != PSA_OPERATION_INCOMPLETE) {
+        if (status != PSA_SUCCESS) {
+            operation->error_occurred = 1;
+        }
+
         psa_verify_hash_abort_internal(operation);
     }
 
@@ -3404,6 +3428,9 @@
      * the operation fails or succeeds, only on manual abort. */
     operation->num_ops = 0;
 
+    /* Likewise, failure state. */
+    operation->error_occurred = 0;
+
     return status;
 }