2020-05-27 22:19:46

by Cabiddu, Giovanni

Subject: [PATCH] crypto: qat - fix parameter check in aead encryption

Return -EINVAL if the input digest size and/or cipher
length is zero or the cipher length is not multiple of a block.
These additional parameter checks prevent an undefined device behaviour.

Signed-off-by: Giovanni Cabiddu <[email protected]>
---
drivers/crypto/qat/qat_common/qat_algs.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/qat/qat_common/qat_algs.c b/drivers/crypto/qat/qat_common/qat_algs.c
index 9df4aebf37ca..00e10a4d578a 100644
--- a/drivers/crypto/qat/qat_common/qat_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_algs.c
@@ -825,8 +825,15 @@ static int qat_alg_aead_dec(struct aead_request *areq)
struct icp_qat_fw_la_auth_req_params *auth_param;
struct icp_qat_fw_la_bulk_req *msg;
int digst_size = crypto_aead_authsize(aead_tfm);
+ u32 cipher_length = areq->cryptlen - digst_size;
int ret, ctr = 0;

+ if (!digst_size || !areq->cryptlen || !cipher_length)
+ return -EINVAL;
+
+ if (cipher_length % AES_BLOCK_SIZE != 0)
+ return -EINVAL;
+
ret = qat_alg_sgl_to_bufl(ctx->inst, areq->src, areq->dst, qat_req);
if (unlikely(ret))
return ret;
@@ -840,7 +847,7 @@ static int qat_alg_aead_dec(struct aead_request *areq)
qat_req->req.comn_mid.src_data_addr = qat_req->buf.blp;
qat_req->req.comn_mid.dest_data_addr = qat_req->buf.bloutp;
cipher_param = (void *)&qat_req->req.serv_specif_rqpars;
- cipher_param->cipher_length = areq->cryptlen - digst_size;
+ cipher_param->cipher_length = cipher_length;
cipher_param->cipher_offset = areq->assoclen;
memcpy(cipher_param->u.cipher_IV_array, areq->iv, AES_BLOCK_SIZE);
auth_param = (void *)((uint8_t *)cipher_param + sizeof(*cipher_param));
@@ -869,6 +876,12 @@ static int qat_alg_aead_enc(struct aead_request *areq)
uint8_t *iv = areq->iv;
int ret, ctr = 0;

+ if (!areq->cryptlen)
+ return -EINVAL;
+
+ if (areq->cryptlen % AES_BLOCK_SIZE != 0)
+ return -EINVAL;
+
ret = qat_alg_sgl_to_bufl(ctx->inst, areq->src, areq->dst, qat_req);
if (unlikely(ret))
return ret;
--
2.26.2

2020-06-12 03:29:32

by Herbert Xu

Subject: Re: [PATCH] crypto: qat - fix parameter check in aead encryption

On Wed, May 27, 2020 at 11:18:52PM +0100, Giovanni Cabiddu wrote:
> Return -EINVAL if the input digest size and/or cipher
> length is zero or the cipher length is not multiple of a block.
> These additional parameter checks prevent an undefined device behaviour.

But a zero-length encryption is valid for AEAD. If this triggers
an issue in the hardware, then you must fallback to software rather
than rejecting it.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt