crypto: s390/phmac - Do not modify the req->nbytes value

The phmac implementation used the req->nbytes field on combined
operations (finup, digest) to track the state:
with req->nbytes > 0 the update needs to be processed,
while req->nbytes == 0 means to do the final operation. For
this purpose the req->nbytes field was set to 0 after successful
update operation. However, aead uses the req->nbytes field after a
successful hash operation to determine the amount of data to
en/decrypt. So an implementation must not modify the nbytes field.

Fixed by a slight rework on the phmac implementation. There is
now a new field async_op in the request context which tracks
the (asynch) operation to process. So the 'state' via req->nbytes
is not needed any more and now this field is untouched and may
be evaluated even after a request is processed by the phmac
implementation.

Fixes: cbbc675506cc ("crypto: s390 - New s390 specific protected key hash phmac")
Reported-by: Ingo Franzki <ifranzki@linux.ibm.com>
Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
Tested-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/arch/s390/crypto/phmac_s390.c b/arch/s390/crypto/phmac_s390.c
index 7ecfdc4fba2d0af09bdbdabbc00a103a682d7f14..89f3e6d8fd897af0238b88c051e2cb20e3f4e35a 100644 (file)
--- a/arch/s390/crypto/phmac_s390.c
+++ b/arch/s390/crypto/phmac_s390.c
@@ -169,11 +169,18 @@ struct kmac_sha2_ctx {
        u64 buflen[2];
 };
 
+enum async_op {
+       OP_NOP = 0,
+       OP_UPDATE,
+       OP_FINAL,
+       OP_FINUP,
+};
+
 /* phmac request context */
 struct phmac_req_ctx {
        struct hash_walk_helper hwh;
        struct kmac_sha2_ctx kmac_ctx;
-       bool final;
+       enum async_op async_op;
 };
 
 /*
@@ -610,6 +617,7 @@ static int phmac_update(struct ahash_request *req)
         * using engine to serialize requests.
         */
        if (rc == 0 || rc == -EKEYEXPIRED) {
+               req_ctx->async_op = OP_UPDATE;
                atomic_inc(&tfm_ctx->via_engine_ctr);
                rc = crypto_transfer_hash_request_to_engine(phmac_crypto_engine, req);
                if (rc != -EINPROGRESS)
@@ -647,8 +655,7 @@ static int phmac_final(struct ahash_request *req)
         * using engine to serialize requests.
         */
        if (rc == 0 || rc == -EKEYEXPIRED) {
-               req->nbytes = 0;
-               req_ctx->final = true;
+               req_ctx->async_op = OP_FINAL;
                atomic_inc(&tfm_ctx->via_engine_ctr);
                rc = crypto_transfer_hash_request_to_engine(phmac_crypto_engine, req);
                if (rc != -EINPROGRESS)
@@ -676,13 +683,16 @@ static int phmac_finup(struct ahash_request *req)
        if (rc)
                goto out;
 
+       req_ctx->async_op = OP_FINUP;
+
        /* Try synchronous operations if no active engine usage */
        if (!atomic_read(&tfm_ctx->via_engine_ctr)) {
                rc = phmac_kmac_update(req, false);
                if (rc == 0)
-                       req->nbytes = 0;
+                       req_ctx->async_op = OP_FINAL;
        }
-       if (!rc && !req->nbytes && !atomic_read(&tfm_ctx->via_engine_ctr)) {
+       if (!rc && req_ctx->async_op == OP_FINAL &&
+           !atomic_read(&tfm_ctx->via_engine_ctr)) {
                rc = phmac_kmac_final(req, false);
                if (rc == 0)
                        goto out;
@@ -694,7 +704,7 @@ static int phmac_finup(struct ahash_request *req)
         * using engine to serialize requests.
         */
        if (rc == 0 || rc == -EKEYEXPIRED) {
-               req_ctx->final = true;
+               /* req->async_op has been set to either OP_FINUP or OP_FINAL */
                atomic_inc(&tfm_ctx->via_engine_ctr);
                rc = crypto_transfer_hash_request_to_engine(phmac_crypto_engine, req);
                if (rc != -EINPROGRESS)
@@ -855,15 +865,16 @@ static int phmac_do_one_request(struct crypto_engine *engine, void *areq)
 
        /*
         * Three kinds of requests come in here:
-        * update when req->nbytes > 0 and req_ctx->final is false
-        * final when req->nbytes = 0 and req_ctx->final is true
-        * finup when req->nbytes > 0 and req_ctx->final is true
-        * For update and finup the hwh walk needs to be prepared and
-        * up to date but the actual nr of bytes in req->nbytes may be
-        * any non zero number. For final there is no hwh walk needed.
+        * 1. req->async_op == OP_UPDATE with req->nbytes > 0
+        * 2. req->async_op == OP_FINUP with req->nbytes > 0
+        * 3. req->async_op == OP_FINAL
+        * For update and finup the hwh walk has already been prepared
+        * by the caller. For final there is no hwh walk needed.
         */
 
-       if (req->nbytes) {
+       switch (req_ctx->async_op) {
+       case OP_UPDATE:
+       case OP_FINUP:
                rc = phmac_kmac_update(req, true);
                if (rc == -EKEYEXPIRED) {
                        /*
@@ -880,10 +891,11 @@ static int phmac_do_one_request(struct crypto_engine *engine, void *areq)
                        hwh_advance(hwh, rc);
                        goto out;
                }
-               req->nbytes = 0;
-       }
-
-       if (req_ctx->final) {
+               if (req_ctx->async_op == OP_UPDATE)
+                       break;
+               req_ctx->async_op = OP_FINAL;
+               fallthrough;
+       case OP_FINAL:
                rc = phmac_kmac_final(req, true);
                if (rc == -EKEYEXPIRED) {
                        /*
@@ -897,10 +909,14 @@ static int phmac_do_one_request(struct crypto_engine *engine, void *areq)
                        cond_resched();
                        return -ENOSPC;
                }
+               break;
+       default:
+               /* unknown/unsupported/unimplemented asynch op */
+               return -EOPNOTSUPP;
        }
 
 out:
-       if (rc || req_ctx->final)
+       if (rc || req_ctx->async_op == OP_FINAL)
                memzero_explicit(kmac_ctx, sizeof(*kmac_ctx));
        pr_debug("request complete with rc=%d\n", rc);
        local_bh_disable();