2021-02-03 14:34:51

by Thara Gopinath

[permalink] [raw]
Subject: [PATCH v4 00/11] Regression fixes/clean ups in the Qualcomm crypto engine driver

This patch series is a result of running kernel crypto fuzz tests (by
enabling CONFIG_CRYPTO_MANAGER_EXTRA_TESTS) on the transformations
currently supported via the Qualcomm crypto engine on sdm845. The first
nine patches are fixes for various regressions found during testing. The
last two patches are minor clean ups of unused variable and parameters.

v3->v4:
- Fixed the bug where only two bytes of byte_count were getting
saved and restored instead of all eight bytes. Thanks Bjorn for
catching this.
- Split patch 3 "Fix regressions found during fuzz testing" into
6 patches as requested by Bjorn.
- Dropped crypto from all subject headers.
- Rebased to 5.11-rc5
v2->v3:
- Made the comparison between keys to check if any two keys are
same for triple des algorithms constant-time as per
Nym Seddon's suggestion.
- Rebased to 5.11-rc4.
v1->v2:
- Introduced custom struct qce_sha_saved_state to store and restore
partial sha transformation.
- Rebased to 5.11-rc3.

Thara Gopinath (11):
crypto: qce: sha: Restore/save ahash state with custom struct in
export/import
crypto: qce: sha: Hold back a block of data to be transferred as part
of final
crypto: qce: skcipher: Return unsupported if key1 and key 2 are same
for AES XTS algorithm
crypto: qce: skcipher: Return unsupported if any three keys are same
for DES3 algorithms
crypto: qce: skcipher: Return error for zero length messages
crypto: qce: skcipher: Return error for non-blocksize data(ECB/CBC
algorithms)
crypto: qce: skcipher: Set ivsize to 0 for ecb(aes)
crypto: qce: skcipher: Improve the conditions for requesting AES
fallback cipher
crypto: qce: common: Set data unit size to message length for AES XTS
transformation
drivers: crypto: qce: Remover src_tbl from qce_cipher_reqctx
crypto: qce: Remove totallen and offset in qce_start

drivers/crypto/qce/cipher.h | 1 -
drivers/crypto/qce/common.c | 25 +++---
drivers/crypto/qce/common.h | 3 +-
drivers/crypto/qce/sha.c | 143 +++++++++++++---------------------
drivers/crypto/qce/skcipher.c | 72 ++++++++++++++---
5 files changed, 129 insertions(+), 115 deletions(-)

--
2.25.1


2021-02-03 14:37:19

by Thara Gopinath

[permalink] [raw]
Subject: [PATCH v4 02/11] crypto: qce: sha: Hold back a block of data to be transferred as part of final

If the available data to transfer is exactly a multiple of block size, save
the last block to be transferred in qce_ahash_final (with the last block
bit set) if this is indeed the end of data stream. If not this saved block
will be transferred as part of next update. If this block is not held back
and if this is indeed the end of data stream, the digest obtained will be
wrong since qce_ahash_final will see that rctx->buflen is 0 and return
doing nothing which in turn means that a digest will not be copied to the
destination result buffer. qce_ahash_final cannot be made to alter this
behavior and allowed to proceed if rctx->buflen is 0 because the crypto
engine BAM does not allow for zero length transfers.

Reviewed-by: Bjorn Andersson <[email protected]>
Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/crypto/qce/sha.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/drivers/crypto/qce/sha.c b/drivers/crypto/qce/sha.c
index 500290b40916..c8bfa9db07b8 100644
--- a/drivers/crypto/qce/sha.c
+++ b/drivers/crypto/qce/sha.c
@@ -216,6 +216,25 @@ static int qce_ahash_update(struct ahash_request *req)

/* calculate how many bytes will be hashed later */
hash_later = total % blocksize;
+
+ /*
+ * At this point, there is more than one block size of data. If
+ * the available data to transfer is exactly a multiple of block
+ * size, save the last block to be transferred in qce_ahash_final
+ * (with the last block bit set) if this is indeed the end of data
+ * stream. If not this saved block will be transferred as part of
+ * next update. If this block is not held back and if this is
+ * indeed the end of data stream, the digest obtained will be wrong
+ * since qce_ahash_final will see that rctx->buflen is 0 and return
+ * doing nothing which in turn means that a digest will not be
+ * copied to the destination result buffer. qce_ahash_final cannot
+ * be made to alter this behavior and allowed to proceed if
+ * rctx->buflen is 0 because the crypto engine BAM does not allow
+ * for zero length transfers.
+ */
+ if (!hash_later)
+ hash_later = blocksize;
+
if (hash_later) {
unsigned int src_offset = req->nbytes - hash_later;
scatterwalk_map_and_copy(rctx->buf, req->src, src_offset,
--
2.25.1

2021-02-03 14:37:51

by Thara Gopinath

[permalink] [raw]
Subject: [PATCH v4 03/11] crypto: qce: skcipher: Return unsupported if key1 and key 2 are same for AES XTS algorithm

Crypto engine does not support key1 = key2 for AES XTS algorithm; the
operation hangs the engines. Return -EINVAL in case key1 and key2 are the
same.

Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/crypto/qce/skcipher.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/qce/skcipher.c b/drivers/crypto/qce/skcipher.c
index a2d3da0ad95f..12955dcd53dd 100644
--- a/drivers/crypto/qce/skcipher.c
+++ b/drivers/crypto/qce/skcipher.c
@@ -167,16 +167,33 @@ static int qce_skcipher_setkey(struct crypto_skcipher *ablk, const u8 *key,
struct crypto_tfm *tfm = crypto_skcipher_tfm(ablk);
struct qce_cipher_ctx *ctx = crypto_tfm_ctx(tfm);
unsigned long flags = to_cipher_tmpl(ablk)->alg_flags;
+ unsigned int __keylen;
int ret;

if (!key || !keylen)
return -EINVAL;

- switch (IS_XTS(flags) ? keylen >> 1 : keylen) {
+ /*
+ * AES XTS key1 = key2 not supported by crypto engine.
+ * Revisit to request a fallback cipher in this case.
+ */
+ if (IS_XTS(flags)) {
+ __keylen = keylen >> 1;
+ if (!memcmp(key, key + __keylen, __keylen))
+ return -ENOKEY;
+ } else {
+ __keylen = keylen;
+ }
+
+ switch (__keylen) {
case AES_KEYSIZE_128:
case AES_KEYSIZE_256:
memcpy(ctx->enc_key, key, keylen);
break;
+ case AES_KEYSIZE_192:
+ break;
+ default:
+ return -EINVAL;
}

ret = crypto_skcipher_setkey(ctx->fallback, key, keylen);
--
2.25.1