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.
v6->v7:
- Fixed sparse warning in patch 4 as pointed out by Herbert Xu.
This means the checking if any two keys are same for triple
des algorithms has been reverted back to using conditional OR
instead of using bitwise OR.
- Rebased to 5.11-rc7.
v5->v6:
- Return 0 for zero length messages instead of -EOPNOTSUPP in the
cipher algorithms as pointed out by Eric Biggers.
- Remove the wrong TODO in patch 6 which implied that AES CBC can
do partial block sizes when it is actually CTS mode that can as
pointed out my Eric Biggers.
v4->v5:
- Fixed build warning/error in patch for wrong assignment of const
pointer as reported by kernel test robot <[email protected]>.
- Rebased to 5.11-rc6.
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)
*** BLURB HERE ***
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
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 | 69 +++++++++++++---
5 files changed, 126 insertions(+), 115 deletions(-)
--
2.25.1
Crypto engine BAM dma does not support 0 length data. Return unsupported
if zero length messages are passed for transformation.
Signed-off-by: Thara Gopinath <[email protected]>
---
v5->v6:
- Return 0 for zero length messages instead of -EOPNOTSUPP in the
cipher algorithms as pointed out by Eric Biggers.
drivers/crypto/qce/skcipher.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/crypto/qce/skcipher.c b/drivers/crypto/qce/skcipher.c
index 8aeb741ca5a3..6b3dc3a9797c 100644
--- a/drivers/crypto/qce/skcipher.c
+++ b/drivers/crypto/qce/skcipher.c
@@ -8,6 +8,7 @@
#include <linux/interrupt.h>
#include <linux/moduleparam.h>
#include <linux/types.h>
+#include <linux/errno.h>
#include <crypto/aes.h>
#include <crypto/internal/des.h>
#include <crypto/internal/skcipher.h>
@@ -260,6 +261,10 @@ static int qce_skcipher_crypt(struct skcipher_request *req, int encrypt)
rctx->flags |= encrypt ? QCE_ENCRYPT : QCE_DECRYPT;
keylen = IS_XTS(rctx->flags) ? ctx->enc_keylen >> 1 : ctx->enc_keylen;
+ /* CE does not handle 0 length messages */
+ if (!req->cryptlen)
+ return 0;
+
/* qce is hanging when AES-XTS request len > QCE_SECTOR_SIZE and
* is not a multiple of it; pass such requests to the fallback
*/
--
2.25.1
ECB/CBC encryption/decryption requires the data to be blocksize aligned.
Crypto engine hangs on non-block sized operations for these algorithms.
Return invalid data if data size is not blocksize aligned for these
algorithms.
Signed-off-by: Thara Gopinath <[email protected]>
---
v5->v6:
- Remove the wrong TODO which implied that AES CBC can do partial
block sizes when it is actually CTS mode that can as pointed
out by Eric Biggers.
drivers/crypto/qce/skcipher.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/crypto/qce/skcipher.c b/drivers/crypto/qce/skcipher.c
index 6b3dc3a9797c..c2f0469ffb22 100644
--- a/drivers/crypto/qce/skcipher.c
+++ b/drivers/crypto/qce/skcipher.c
@@ -254,6 +254,7 @@ static int qce_skcipher_crypt(struct skcipher_request *req, int encrypt)
struct qce_cipher_ctx *ctx = crypto_skcipher_ctx(tfm);
struct qce_cipher_reqctx *rctx = skcipher_request_ctx(req);
struct qce_alg_template *tmpl = to_cipher_tmpl(tfm);
+ unsigned int blocksize = crypto_skcipher_blocksize(tfm);
int keylen;
int ret;
@@ -265,6 +266,14 @@ static int qce_skcipher_crypt(struct skcipher_request *req, int encrypt)
if (!req->cryptlen)
return 0;
+ /*
+ * ECB and CBC algorithms require message lengths to be
+ * multiples of block size.
+ */
+ if (IS_ECB(rctx->flags) || IS_CBC(rctx->flags))
+ if (!IS_ALIGNED(req->cryptlen, blocksize))
+ return -EINVAL;
+
/* qce is hanging when AES-XTS request len > QCE_SECTOR_SIZE and
* is not a multiple of it; pass such requests to the fallback
*/
--
2.25.1
ECB transformations do not have an IV and hence set the ivsize to 0 for
ecb(aes).
Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/crypto/qce/skcipher.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/qce/skcipher.c b/drivers/crypto/qce/skcipher.c
index c2f0469ffb22..11a2a30631af 100644
--- a/drivers/crypto/qce/skcipher.c
+++ b/drivers/crypto/qce/skcipher.c
@@ -353,7 +353,7 @@ static const struct qce_skcipher_def skcipher_def[] = {
.name = "ecb(aes)",
.drv_name = "ecb-aes-qce",
.blocksize = AES_BLOCK_SIZE,
- .ivsize = AES_BLOCK_SIZE,
+ .ivsize = 0,
.min_keysize = AES_MIN_KEY_SIZE,
.max_keysize = AES_MAX_KEY_SIZE,
},
--
2.25.1
totallen is used to get the size of the data to be transformed.
This is also available via nbytes or cryptlen in the qce_sha_reqctx
and qce_cipher_ctx. Similarly offset convey nothing for the supported
encryption and authentication transformations and is always 0.
Remove these two redundant parameters in qce_start.
Reviewed-by: Bjorn Andersson <[email protected]>
Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/crypto/qce/common.c | 17 +++++++----------
drivers/crypto/qce/common.h | 3 +--
drivers/crypto/qce/sha.c | 2 +-
drivers/crypto/qce/skcipher.c | 2 +-
4 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c
index f7bc701a4aa2..dceb9579d87a 100644
--- a/drivers/crypto/qce/common.c
+++ b/drivers/crypto/qce/common.c
@@ -140,8 +140,7 @@ static u32 qce_auth_cfg(unsigned long flags, u32 key_size)
return cfg;
}
-static int qce_setup_regs_ahash(struct crypto_async_request *async_req,
- u32 totallen, u32 offset)
+static int qce_setup_regs_ahash(struct crypto_async_request *async_req)
{
struct ahash_request *req = ahash_request_cast(async_req);
struct crypto_ahash *ahash = __crypto_ahash_cast(async_req->tfm);
@@ -306,8 +305,7 @@ static void qce_xtskey(struct qce_device *qce, const u8 *enckey,
qce_write(qce, REG_ENCR_XTS_DU_SIZE, cryptlen);
}
-static int qce_setup_regs_skcipher(struct crypto_async_request *async_req,
- u32 totallen, u32 offset)
+static int qce_setup_regs_skcipher(struct crypto_async_request *async_req)
{
struct skcipher_request *req = skcipher_request_cast(async_req);
struct qce_cipher_reqctx *rctx = skcipher_request_ctx(req);
@@ -367,7 +365,7 @@ static int qce_setup_regs_skcipher(struct crypto_async_request *async_req,
qce_write(qce, REG_ENCR_SEG_CFG, encr_cfg);
qce_write(qce, REG_ENCR_SEG_SIZE, rctx->cryptlen);
- qce_write(qce, REG_ENCR_SEG_START, offset & 0xffff);
+ qce_write(qce, REG_ENCR_SEG_START, 0);
if (IS_CTR(flags)) {
qce_write(qce, REG_CNTR_MASK, ~0);
@@ -376,7 +374,7 @@ static int qce_setup_regs_skcipher(struct crypto_async_request *async_req,
qce_write(qce, REG_CNTR_MASK2, ~0);
}
- qce_write(qce, REG_SEG_SIZE, totallen);
+ qce_write(qce, REG_SEG_SIZE, rctx->cryptlen);
/* get little endianness */
config = qce_config_reg(qce, 1);
@@ -388,17 +386,16 @@ static int qce_setup_regs_skcipher(struct crypto_async_request *async_req,
}
#endif
-int qce_start(struct crypto_async_request *async_req, u32 type, u32 totallen,
- u32 offset)
+int qce_start(struct crypto_async_request *async_req, u32 type)
{
switch (type) {
#ifdef CONFIG_CRYPTO_DEV_QCE_SKCIPHER
case CRYPTO_ALG_TYPE_SKCIPHER:
- return qce_setup_regs_skcipher(async_req, totallen, offset);
+ return qce_setup_regs_skcipher(async_req);
#endif
#ifdef CONFIG_CRYPTO_DEV_QCE_SHA
case CRYPTO_ALG_TYPE_AHASH:
- return qce_setup_regs_ahash(async_req, totallen, offset);
+ return qce_setup_regs_ahash(async_req);
#endif
default:
return -EINVAL;
diff --git a/drivers/crypto/qce/common.h b/drivers/crypto/qce/common.h
index 85ba16418a04..3bc244bcca2d 100644
--- a/drivers/crypto/qce/common.h
+++ b/drivers/crypto/qce/common.h
@@ -94,7 +94,6 @@ struct qce_alg_template {
void qce_cpu_to_be32p_array(__be32 *dst, const u8 *src, unsigned int len);
int qce_check_status(struct qce_device *qce, u32 *status);
void qce_get_version(struct qce_device *qce, u32 *major, u32 *minor, u32 *step);
-int qce_start(struct crypto_async_request *async_req, u32 type, u32 totallen,
- u32 offset);
+int qce_start(struct crypto_async_request *async_req, u32 type);
#endif /* _COMMON_H_ */
diff --git a/drivers/crypto/qce/sha.c b/drivers/crypto/qce/sha.c
index 2813c9a27a6e..8e6fcf2c21cc 100644
--- a/drivers/crypto/qce/sha.c
+++ b/drivers/crypto/qce/sha.c
@@ -113,7 +113,7 @@ static int qce_ahash_async_req_handle(struct crypto_async_request *async_req)
qce_dma_issue_pending(&qce->dma);
- ret = qce_start(async_req, tmpl->crypto_alg_type, 0, 0);
+ ret = qce_start(async_req, tmpl->crypto_alg_type);
if (ret)
goto error_terminate;
diff --git a/drivers/crypto/qce/skcipher.c b/drivers/crypto/qce/skcipher.c
index 2e6ab1d33a31..c0a0d8c4fce1 100644
--- a/drivers/crypto/qce/skcipher.c
+++ b/drivers/crypto/qce/skcipher.c
@@ -144,7 +144,7 @@ qce_skcipher_async_req_handle(struct crypto_async_request *async_req)
qce_dma_issue_pending(&qce->dma);
- ret = qce_start(async_req, tmpl->crypto_alg_type, req->cryptlen, 0);
+ ret = qce_start(async_req, tmpl->crypto_alg_type);
if (ret)
goto error_terminate;
--
2.25.1
Return unsupported if any three keys are same for DES3 algorithms
since CE does not support this and the operation causes the engine to
hang.
Signed-off-by: Thara Gopinath <[email protected]>
---
v6->v7:
- Fixed sparse warning in patch 4 as pointed out by Herbert Xu.
This means the checking if any two keys are same for triple
des algorithms has been reverted back to using conditional OR
instead of using bitwise OR.
drivers/crypto/qce/skcipher.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/crypto/qce/skcipher.c b/drivers/crypto/qce/skcipher.c
index 12955dcd53dd..8aeb741ca5a3 100644
--- a/drivers/crypto/qce/skcipher.c
+++ b/drivers/crypto/qce/skcipher.c
@@ -221,12 +221,27 @@ static int qce_des3_setkey(struct crypto_skcipher *ablk, const u8 *key,
unsigned int keylen)
{
struct qce_cipher_ctx *ctx = crypto_skcipher_ctx(ablk);
+ u32 _key[6];
int err;
err = verify_skcipher_des3_key(ablk, key);
if (err)
return err;
+ /*
+ * The crypto engine does not support any two keys
+ * being the same for triple des algorithms. The
+ * verify_skcipher_des3_key does not check for all the
+ * below conditions. Return -ENOKEY in case any two keys
+ * are the same. Revisit to see if a fallback cipher
+ * is needed to handle this condition.
+ */
+ memcpy(_key, key, DES3_EDE_KEY_SIZE);
+ if (!((_key[0] ^ _key[2]) | (_key[1] ^ _key[3])) ||
+ !((_key[2] ^ _key[4]) | (_key[3] ^ _key[5])) ||
+ !((_key[0] ^ _key[4]) | (_key[1] ^ _key[5])))
+ return -ENOKEY;
+
ctx->enc_keylen = keylen;
memcpy(ctx->enc_key, key, keylen);
return 0;
--
2.25.1
src_table is unused and hence remove it from struct qce_cipher_reqctx
Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/crypto/qce/cipher.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/crypto/qce/cipher.h b/drivers/crypto/qce/cipher.h
index cffa9fc628ff..850f257d00f3 100644
--- a/drivers/crypto/qce/cipher.h
+++ b/drivers/crypto/qce/cipher.h
@@ -40,7 +40,6 @@ struct qce_cipher_reqctx {
struct scatterlist result_sg;
struct sg_table dst_tbl;
struct scatterlist *dst_sg;
- struct sg_table src_tbl;
struct scatterlist *src_sg;
unsigned int cryptlen;
struct skcipher_request fallback_req; // keep at the end
--
2.25.1
The following are the conditions for requesting AES fallback cipher.
- AES-192
- AES-XTS request with len <= 512 byte (Allow messages of length
less than 512 bytes for all other AES encryption algorithms other
than AES XTS)
- AES-XTS request with len > QCE_SECTOR_SIZE and is not a multiple
of it
Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/crypto/qce/skcipher.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/crypto/qce/skcipher.c b/drivers/crypto/qce/skcipher.c
index 11a2a30631af..2e6ab1d33a31 100644
--- a/drivers/crypto/qce/skcipher.c
+++ b/drivers/crypto/qce/skcipher.c
@@ -274,14 +274,19 @@ static int qce_skcipher_crypt(struct skcipher_request *req, int encrypt)
if (!IS_ALIGNED(req->cryptlen, blocksize))
return -EINVAL;
- /* qce is hanging when AES-XTS request len > QCE_SECTOR_SIZE and
- * is not a multiple of it; pass such requests to the fallback
+ /*
+ * Conditions for requesting a fallback cipher
+ * AES-192 (not supported by crypto engine (CE))
+ * AES-XTS request with len <= 512 byte (not recommended to use CE)
+ * AES-XTS request with len > QCE_SECTOR_SIZE and
+ * is not a multiple of it.(Revisit this condition to check if it is
+ * needed in all versions of CE)
*/
if (IS_AES(rctx->flags) &&
- (((keylen != AES_KEYSIZE_128 && keylen != AES_KEYSIZE_256) ||
- req->cryptlen <= aes_sw_max_len) ||
- (IS_XTS(rctx->flags) && req->cryptlen > QCE_SECTOR_SIZE &&
- req->cryptlen % QCE_SECTOR_SIZE))) {
+ ((keylen != AES_KEYSIZE_128 && keylen != AES_KEYSIZE_256) ||
+ (IS_XTS(rctx->flags) && ((req->cryptlen <= aes_sw_max_len) ||
+ (req->cryptlen > QCE_SECTOR_SIZE &&
+ req->cryptlen % QCE_SECTOR_SIZE))))) {
skcipher_request_set_tfm(&rctx->fallback_req, ctx->fallback);
skcipher_request_set_callback(&rctx->fallback_req,
req->base.flags,
--
2.25.1
Set the register REG_ENCR_XTS_DU_SIZE to cryptlen for AES XTS
transformation. Anything else causes the engine to return back
wrong results.
Acked-by: Bjorn Andersson <[email protected]>
Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/crypto/qce/common.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c
index a73db2a5637f..f7bc701a4aa2 100644
--- a/drivers/crypto/qce/common.c
+++ b/drivers/crypto/qce/common.c
@@ -295,15 +295,15 @@ static void qce_xtskey(struct qce_device *qce, const u8 *enckey,
{
u32 xtskey[QCE_MAX_CIPHER_KEY_SIZE / sizeof(u32)] = {0};
unsigned int xtsklen = enckeylen / (2 * sizeof(u32));
- unsigned int xtsdusize;
qce_cpu_to_be32p_array((__be32 *)xtskey, enckey + enckeylen / 2,
enckeylen / 2);
qce_write_array(qce, REG_ENCR_XTS_KEY0, xtskey, xtsklen);
- /* xts du size 512B */
- xtsdusize = min_t(u32, QCE_SECTOR_SIZE, cryptlen);
- qce_write(qce, REG_ENCR_XTS_DU_SIZE, xtsdusize);
+ /* Set data unit size to cryptlen. Anything else causes
+ * crypto engine to return back incorrect results.
+ */
+ qce_write(qce, REG_ENCR_XTS_DU_SIZE, cryptlen);
}
static int qce_setup_regs_skcipher(struct crypto_async_request *async_req,
--
2.25.1
On 2/11/21 3:01 PM, Thara Gopinath wrote:
> 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.
Hi Herbert,
This version has all the comments from you and rest of the community
fixed. Do you think you can merge this ?
>
> v6->v7:
> - Fixed sparse warning in patch 4 as pointed out by Herbert Xu.
> This means the checking if any two keys are same for triple
> des algorithms has been reverted back to using conditional OR
> instead of using bitwise OR.
> - Rebased to 5.11-rc7.
>
> v5->v6:
> - Return 0 for zero length messages instead of -EOPNOTSUPP in the
> cipher algorithms as pointed out by Eric Biggers.
> - Remove the wrong TODO in patch 6 which implied that AES CBC can
> do partial block sizes when it is actually CTS mode that can as
> pointed out my Eric Biggers.
>
> v4->v5:
> - Fixed build warning/error in patch for wrong assignment of const
> pointer as reported by kernel test robot <[email protected]>.
> - Rebased to 5.11-rc6.
> 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)
> *** BLURB HERE ***
>
> 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
> 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 | 69 +++++++++++++---
> 5 files changed, 126 insertions(+), 115 deletions(-)
>
--
Warm Regards
Thara