[BUG 1/2] crypto: ahash - import/export fails for algs using CRYPTO_AHASH_ALG_BLOCK_ONLY

[T Pratham]

Hi,

Commit 9d7a0ab1c7536 ("crypto: ahash - Handle partial blocks in API")
introduced partial block handling for ahashes in the crypto API layer itself.
This enables ahash algorithms to return a positive integer from the update
function to indicate the number of bytes in the input which are not processed
and should be buffered for next update/finup/final call to process.

When CRYPTO_AHASH_ALG_BLOCK_ONLY is enabled to let crypto layer handle the
buffering for ahashes, it appears that the import/export for ahashes is broken.
Below are dmesg logs from my (work-in-progress) TI dthev2 hash driver:

[   12.713654] alg: ahash: sha256-dthev2 test failed (wrong result) on test vector 1, cfg="import/export"
[   12.735778] alg: ahash: sha512-dthev2 test failed (wrong result) on test vector 1, cfg="import/export"
[   12.740207] alg: self-tests for sha256 using sha256-dthev2 failed (rc=-22)
[   12.752055] alg: self-tests for sha256 using sha256-dthev2 failed (rc=-22)
[   12.760127] alg: self-tests for sha512 using sha512-dthev2 failed (rc=-22)
[   12.847079] alg: self-tests for sha512 using sha512-dthev2 failed (rc=-22)
[   13.632318] alg: ahash: md5-dthev2 test failed (wrong result) on test vector 2, cfg="import/export"
[   13.643908] alg: self-tests for md5 using md5-dthev2 failed (rc=-22)
[   13.655051] alg: self-tests for md5 using md5-dthev2 failed (rc=-22)

While debugging, I noticed the rather odd asymmetrical handling of the buffer
which stores the partial block in crypto_ahash_export vs crypto_ahash_import in
crypto/ahash.c: 

Export:
> if (crypto_ahash_block_only(tfm)) {
> 		unsigned int plen = crypto_ahash_blocksize(tfm) + 1;
> 		unsigned int reqsize = crypto_ahash_reqsize(tfm);
> 		unsigned int ss = crypto_ahash_statesize(tfm);
> 		u8 *buf = ahash_request_ctx(req);
> 
> 		memcpy(out + ss - plen, buf + reqsize - plen, plen);
> 	}

Import:
> if (crypto_ahash_block_only(tfm)) {
> 		unsigned int reqsize = crypto_ahash_reqsize(tfm);
> 		u8 *buf = ahash_request_ctx(req);
> 
> 		buf[reqsize - 1] = 0;
> 	}

Import seemingly not copying the buffer back to request ctx and zeroing the
length seems incorrect. Making export and import symmetrical seems to work and
my driver passes through import/export tests.

diff --git a/crypto/ahash.c b/crypto/ahash.c
index dfb4f5476428f..9510bdeda51de 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -674,10 +674,12 @@ int crypto_ahash_import(struct ahash_request *req, const void *in)
        if (crypto_ahash_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
                return -ENOKEY;
        if (crypto_ahash_block_only(tfm)) {
+               unsigned int plen = crypto_ahash_blocksize(tfm) + 1;
                unsigned int reqsize = crypto_ahash_reqsize(tfm);
+               unsigned int ss = crypto_ahash_statesize(tfm);
                u8 *buf = ahash_request_ctx(req);
 
-               buf[reqsize - 1] = 0;
+               memcpy(buf + reqsize - plen, in + ss - plen, plen);
        }
        return crypto_ahash_alg(tfm)->import(req, in);
 }

Is there any particular reason why import is like how it is currently? As per
my understanding import should reverse whatever export is doing and vice-versa.

It is also noteworthy that similarly import/export could be broken for shash
algorithms as well. There also, import function zeros the length byte and does
not copy the buffer when CRYPTO_AHASH_ALG_BLOCK_ONLY is used. However I haven't
tested any shash algorithm to verify this.

Let me know if the above is the complete fix for ahash, so that I can send it
as a patch.

-- 
Regards
T Pratham <t-pratham@xxxxxx>