linux-crypto.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] crypto: qat - fixes to aes xts
@ 2020-06-26  8:04 Giovanni Cabiddu
  2020-06-26  8:04 ` [PATCH v2 1/4] crypto: qat - allow xts requests not multiple of block Giovanni Cabiddu
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Giovanni Cabiddu @ 2020-06-26  8:04 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto, qat-linux, Giovanni Cabiddu

This series fixes a few issues with the xts(aes) implementation in the
QuickAssist driver:
 - Requests that are not multiple of the block size are rejected
 - Input key not validated
 - xts(aes) requests with key size 192 bits are rejected with -EINVAL

Changes from v1:
 - Removed extra pair of parenthesis around PTR_ERR in patch #4 (crypto:
   qat - allow xts requests not multiple of block)

Giovanni Cabiddu (4):
  crypto: qat - allow xts requests not multiple of block
  crypto: qat - validate xts key
  crypto: qat - remove unused field in skcipher ctx
  crypto: qat - fallback for xts with 192 bit keys

 drivers/crypto/qat/qat_common/qat_algs.c | 97 ++++++++++++++++++++++--
 1 file changed, 89 insertions(+), 8 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/4] crypto: qat - allow xts requests not multiple of block
  2020-06-26  8:04 [PATCH v2 0/4] crypto: qat - fixes to aes xts Giovanni Cabiddu
@ 2020-06-26  8:04 ` Giovanni Cabiddu
  2020-06-26  8:04 ` [PATCH v2 2/4] crypto: qat - validate xts key Giovanni Cabiddu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Giovanni Cabiddu @ 2020-06-26  8:04 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto, qat-linux, Giovanni Cabiddu

Allow aes xts requests that are not multiple of the block size.
If a request is smaller than the block size, return -EINVAL.

This fixes the following issue reported by the crypto testmgr self-test:
alg: skcipher: qat_aes_xts encryption failed on test vector "random:
len=116 klen=64"; expected_error=0, actual_error=-22, cfg="random:
inplace may_sleep use_finup src_divs=[<reimport>45.85%@+4077,
<flush>54.15%@alignmask+18]"

Fixes: 96ee111a659e ("crypto: qat - return error for block...")
Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
 drivers/crypto/qat/qat_common/qat_algs.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/qat/qat_common/qat_algs.c b/drivers/crypto/qat/qat_common/qat_algs.c
index 833d1a75666f..6bea6f868395 100644
--- a/drivers/crypto/qat/qat_common/qat_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_algs.c
@@ -11,6 +11,7 @@
 #include <crypto/hmac.h>
 #include <crypto/algapi.h>
 #include <crypto/authenc.h>
+#include <crypto/xts.h>
 #include <linux/dma-mapping.h>
 #include "adf_accel_devices.h"
 #include "adf_transport.h"
@@ -1058,6 +1059,14 @@ static int qat_alg_skcipher_blk_encrypt(struct skcipher_request *req)
 	return qat_alg_skcipher_encrypt(req);
 }
 
+static int qat_alg_skcipher_xts_encrypt(struct skcipher_request *req)
+{
+	if (req->cryptlen < XTS_BLOCK_SIZE)
+		return -EINVAL;
+
+	return qat_alg_skcipher_encrypt(req);
+}
+
 static int qat_alg_skcipher_decrypt(struct skcipher_request *req)
 {
 	struct crypto_skcipher *stfm = crypto_skcipher_reqtfm(req);
@@ -1117,6 +1126,15 @@ static int qat_alg_skcipher_blk_decrypt(struct skcipher_request *req)
 
 	return qat_alg_skcipher_decrypt(req);
 }
+
+static int qat_alg_skcipher_xts_decrypt(struct skcipher_request *req)
+{
+	if (req->cryptlen < XTS_BLOCK_SIZE)
+		return -EINVAL;
+
+	return qat_alg_skcipher_decrypt(req);
+}
+
 static int qat_alg_aead_init(struct crypto_aead *tfm,
 			     enum icp_qat_hw_auth_algo hash,
 			     const char *hash_name)
@@ -1310,8 +1328,8 @@ static struct skcipher_alg qat_skciphers[] = { {
 	.init = qat_alg_skcipher_init_tfm,
 	.exit = qat_alg_skcipher_exit_tfm,
 	.setkey = qat_alg_skcipher_xts_setkey,
-	.decrypt = qat_alg_skcipher_blk_decrypt,
-	.encrypt = qat_alg_skcipher_blk_encrypt,
+	.decrypt = qat_alg_skcipher_xts_decrypt,
+	.encrypt = qat_alg_skcipher_xts_encrypt,
 	.min_keysize = 2 * AES_MIN_KEY_SIZE,
 	.max_keysize = 2 * AES_MAX_KEY_SIZE,
 	.ivsize = AES_BLOCK_SIZE,
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/4] crypto: qat - validate xts key
  2020-06-26  8:04 [PATCH v2 0/4] crypto: qat - fixes to aes xts Giovanni Cabiddu
  2020-06-26  8:04 ` [PATCH v2 1/4] crypto: qat - allow xts requests not multiple of block Giovanni Cabiddu
@ 2020-06-26  8:04 ` Giovanni Cabiddu
  2020-06-26  8:04 ` [PATCH v2 3/4] crypto: qat - remove unused field in skcipher ctx Giovanni Cabiddu
  2020-06-26  8:04 ` [PATCH v2 4/4] crypto: qat - fallback for xts with 192 bit keys Giovanni Cabiddu
  3 siblings, 0 replies; 8+ messages in thread
From: Giovanni Cabiddu @ 2020-06-26  8:04 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto, qat-linux, Giovanni Cabiddu

Validate xts key using the function xts_verify_key() to prevent
malformed keys.

Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
 drivers/crypto/qat/qat_common/qat_algs.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/crypto/qat/qat_common/qat_algs.c b/drivers/crypto/qat/qat_common/qat_algs.c
index 6bea6f868395..11f36eafda0c 100644
--- a/drivers/crypto/qat/qat_common/qat_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_algs.c
@@ -995,6 +995,12 @@ static int qat_alg_skcipher_ctr_setkey(struct crypto_skcipher *tfm,
 static int qat_alg_skcipher_xts_setkey(struct crypto_skcipher *tfm,
 				       const u8 *key, unsigned int keylen)
 {
+	int ret;
+
+	ret = xts_verify_key(tfm, key, keylen);
+	if (ret)
+		return ret;
+
 	return qat_alg_skcipher_setkey(tfm, key, keylen,
 				       ICP_QAT_HW_CIPHER_XTS_MODE);
 }
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/4] crypto: qat - remove unused field in skcipher ctx
  2020-06-26  8:04 [PATCH v2 0/4] crypto: qat - fixes to aes xts Giovanni Cabiddu
  2020-06-26  8:04 ` [PATCH v2 1/4] crypto: qat - allow xts requests not multiple of block Giovanni Cabiddu
  2020-06-26  8:04 ` [PATCH v2 2/4] crypto: qat - validate xts key Giovanni Cabiddu
@ 2020-06-26  8:04 ` Giovanni Cabiddu
  2020-06-26  8:04 ` [PATCH v2 4/4] crypto: qat - fallback for xts with 192 bit keys Giovanni Cabiddu
  3 siblings, 0 replies; 8+ messages in thread
From: Giovanni Cabiddu @ 2020-06-26  8:04 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto, qat-linux, Giovanni Cabiddu

Remove tfm field in qat_alg_skcipher_ctx structure.
This is not used.

Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
 drivers/crypto/qat/qat_common/qat_algs.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/crypto/qat/qat_common/qat_algs.c b/drivers/crypto/qat/qat_common/qat_algs.c
index 11f36eafda0c..77bdff0118f7 100644
--- a/drivers/crypto/qat/qat_common/qat_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_algs.c
@@ -88,7 +88,6 @@ struct qat_alg_skcipher_ctx {
 	struct icp_qat_fw_la_bulk_req enc_fw_req;
 	struct icp_qat_fw_la_bulk_req dec_fw_req;
 	struct qat_crypto_instance *inst;
-	struct crypto_skcipher *tfm;
 };
 
 static int qat_get_inter_state_size(enum icp_qat_hw_auth_algo qat_hash_alg)
@@ -1197,10 +1196,7 @@ static void qat_alg_aead_exit(struct crypto_aead *tfm)
 
 static int qat_alg_skcipher_init_tfm(struct crypto_skcipher *tfm)
 {
-	struct qat_alg_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
-
 	crypto_skcipher_set_reqsize(tfm, sizeof(struct qat_crypto_request));
-	ctx->tfm = tfm;
 	return 0;
 }
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 4/4] crypto: qat - fallback for xts with 192 bit keys
  2020-06-26  8:04 [PATCH v2 0/4] crypto: qat - fixes to aes xts Giovanni Cabiddu
                   ` (2 preceding siblings ...)
  2020-06-26  8:04 ` [PATCH v2 3/4] crypto: qat - remove unused field in skcipher ctx Giovanni Cabiddu
@ 2020-06-26  8:04 ` Giovanni Cabiddu
  2020-06-26 18:15   ` Ard Biesheuvel
  3 siblings, 1 reply; 8+ messages in thread
From: Giovanni Cabiddu @ 2020-06-26  8:04 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto, qat-linux, Giovanni Cabiddu

Forward requests to another provider if the key length is 192 bits as
this is not supported by the QAT accelerators.

This fixes the following issue reported by the extra self test:
alg: skcipher: qat_aes_xts setkey failed on test vector "random: len=3204
klen=48"; expected_error=0, actual_error=-22, flags=0x1

Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
 drivers/crypto/qat/qat_common/qat_algs.c | 67 ++++++++++++++++++++++--
 1 file changed, 64 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/qat/qat_common/qat_algs.c b/drivers/crypto/qat/qat_common/qat_algs.c
index 77bdff0118f7..5e8c0b6f2834 100644
--- a/drivers/crypto/qat/qat_common/qat_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_algs.c
@@ -88,6 +88,8 @@ struct qat_alg_skcipher_ctx {
 	struct icp_qat_fw_la_bulk_req enc_fw_req;
 	struct icp_qat_fw_la_bulk_req dec_fw_req;
 	struct qat_crypto_instance *inst;
+	struct crypto_skcipher *ftfm;
+	bool fallback;
 };
 
 static int qat_get_inter_state_size(enum icp_qat_hw_auth_algo qat_hash_alg)
@@ -994,12 +996,25 @@ static int qat_alg_skcipher_ctr_setkey(struct crypto_skcipher *tfm,
 static int qat_alg_skcipher_xts_setkey(struct crypto_skcipher *tfm,
 				       const u8 *key, unsigned int keylen)
 {
+	struct qat_alg_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
 	int ret;
 
 	ret = xts_verify_key(tfm, key, keylen);
 	if (ret)
 		return ret;
 
+	if (keylen >> 1 == AES_KEYSIZE_192) {
+		ret = crypto_skcipher_setkey(ctx->ftfm, key, keylen);
+		if (ret)
+			return ret;
+
+		ctx->fallback = true;
+
+		return 0;
+	}
+
+	ctx->fallback = false;
+
 	return qat_alg_skcipher_setkey(tfm, key, keylen,
 				       ICP_QAT_HW_CIPHER_XTS_MODE);
 }
@@ -1066,9 +1081,19 @@ static int qat_alg_skcipher_blk_encrypt(struct skcipher_request *req)
 
 static int qat_alg_skcipher_xts_encrypt(struct skcipher_request *req)
 {
+	struct crypto_skcipher *stfm = crypto_skcipher_reqtfm(req);
+	struct qat_alg_skcipher_ctx *ctx = crypto_skcipher_ctx(stfm);
+	struct skcipher_request *nreq = skcipher_request_ctx(req);
+
 	if (req->cryptlen < XTS_BLOCK_SIZE)
 		return -EINVAL;
 
+	if (ctx->fallback) {
+		memcpy(nreq, req, sizeof(*req));
+		skcipher_request_set_tfm(nreq, ctx->ftfm);
+		return crypto_skcipher_encrypt(nreq);
+	}
+
 	return qat_alg_skcipher_encrypt(req);
 }
 
@@ -1134,9 +1159,19 @@ static int qat_alg_skcipher_blk_decrypt(struct skcipher_request *req)
 
 static int qat_alg_skcipher_xts_decrypt(struct skcipher_request *req)
 {
+	struct crypto_skcipher *stfm = crypto_skcipher_reqtfm(req);
+	struct qat_alg_skcipher_ctx *ctx = crypto_skcipher_ctx(stfm);
+	struct skcipher_request *nreq = skcipher_request_ctx(req);
+
 	if (req->cryptlen < XTS_BLOCK_SIZE)
 		return -EINVAL;
 
+	if (ctx->fallback) {
+		memcpy(nreq, req, sizeof(*req));
+		skcipher_request_set_tfm(nreq, ctx->ftfm);
+		return crypto_skcipher_decrypt(nreq);
+	}
+
 	return qat_alg_skcipher_decrypt(req);
 }
 
@@ -1200,6 +1235,23 @@ static int qat_alg_skcipher_init_tfm(struct crypto_skcipher *tfm)
 	return 0;
 }
 
+static int qat_alg_skcipher_init_xts_tfm(struct crypto_skcipher *tfm)
+{
+	struct qat_alg_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
+	int reqsize;
+
+	ctx->ftfm = crypto_alloc_skcipher("xts(aes)", 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(ctx->ftfm))
+		return PTR_ERR(ctx->ftfm);
+
+	reqsize = max(sizeof(struct qat_crypto_request),
+		      sizeof(struct skcipher_request) +
+		      crypto_skcipher_reqsize(ctx->ftfm));
+	crypto_skcipher_set_reqsize(tfm, reqsize);
+
+	return 0;
+}
+
 static void qat_alg_skcipher_exit_tfm(struct crypto_skcipher *tfm)
 {
 	struct qat_alg_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
@@ -1227,6 +1279,15 @@ static void qat_alg_skcipher_exit_tfm(struct crypto_skcipher *tfm)
 	qat_crypto_put_instance(inst);
 }
 
+static void qat_alg_skcipher_exit_xts_tfm(struct crypto_skcipher *tfm)
+{
+	struct qat_alg_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
+
+	if (ctx->ftfm)
+		crypto_free_skcipher(ctx->ftfm);
+
+	qat_alg_skcipher_exit_tfm(tfm);
+}
 
 static struct aead_alg qat_aeads[] = { {
 	.base = {
@@ -1321,14 +1382,14 @@ static struct skcipher_alg qat_skciphers[] = { {
 	.base.cra_name = "xts(aes)",
 	.base.cra_driver_name = "qat_aes_xts",
 	.base.cra_priority = 4001,
-	.base.cra_flags = CRYPTO_ALG_ASYNC,
+	.base.cra_flags = CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK,
 	.base.cra_blocksize = AES_BLOCK_SIZE,
 	.base.cra_ctxsize = sizeof(struct qat_alg_skcipher_ctx),
 	.base.cra_alignmask = 0,
 	.base.cra_module = THIS_MODULE,
 
-	.init = qat_alg_skcipher_init_tfm,
-	.exit = qat_alg_skcipher_exit_tfm,
+	.init = qat_alg_skcipher_init_xts_tfm,
+	.exit = qat_alg_skcipher_exit_xts_tfm,
 	.setkey = qat_alg_skcipher_xts_setkey,
 	.decrypt = qat_alg_skcipher_xts_decrypt,
 	.encrypt = qat_alg_skcipher_xts_encrypt,
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 4/4] crypto: qat - fallback for xts with 192 bit keys
  2020-06-26  8:04 ` [PATCH v2 4/4] crypto: qat - fallback for xts with 192 bit keys Giovanni Cabiddu
@ 2020-06-26 18:15   ` Ard Biesheuvel
  2020-06-29 17:04     ` Giovanni Cabiddu
  0 siblings, 1 reply; 8+ messages in thread
From: Ard Biesheuvel @ 2020-06-26 18:15 UTC (permalink / raw)
  To: Giovanni Cabiddu; +Cc: Herbert Xu, Linux Crypto Mailing List, qat-linux

On Fri, 26 Jun 2020 at 10:04, Giovanni Cabiddu
<giovanni.cabiddu@intel.com> wrote:
>
> Forward requests to another provider if the key length is 192 bits as
> this is not supported by the QAT accelerators.
>
> This fixes the following issue reported by the extra self test:
> alg: skcipher: qat_aes_xts setkey failed on test vector "random: len=3204
> klen=48"; expected_error=0, actual_error=-22, flags=0x1
>
> Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
> ---
>  drivers/crypto/qat/qat_common/qat_algs.c | 67 ++++++++++++++++++++++--
>  1 file changed, 64 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/crypto/qat/qat_common/qat_algs.c b/drivers/crypto/qat/qat_common/qat_algs.c
> index 77bdff0118f7..5e8c0b6f2834 100644
> --- a/drivers/crypto/qat/qat_common/qat_algs.c
> +++ b/drivers/crypto/qat/qat_common/qat_algs.c
> @@ -88,6 +88,8 @@ struct qat_alg_skcipher_ctx {
>         struct icp_qat_fw_la_bulk_req enc_fw_req;
>         struct icp_qat_fw_la_bulk_req dec_fw_req;
>         struct qat_crypto_instance *inst;
> +       struct crypto_skcipher *ftfm;
> +       bool fallback;
>  };
>
>  static int qat_get_inter_state_size(enum icp_qat_hw_auth_algo qat_hash_alg)
> @@ -994,12 +996,25 @@ static int qat_alg_skcipher_ctr_setkey(struct crypto_skcipher *tfm,
>  static int qat_alg_skcipher_xts_setkey(struct crypto_skcipher *tfm,
>                                        const u8 *key, unsigned int keylen)
>  {
> +       struct qat_alg_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
>         int ret;
>
>         ret = xts_verify_key(tfm, key, keylen);
>         if (ret)
>                 return ret;
>
> +       if (keylen >> 1 == AES_KEYSIZE_192) {
> +               ret = crypto_skcipher_setkey(ctx->ftfm, key, keylen);
> +               if (ret)
> +                       return ret;
> +
> +               ctx->fallback = true;
> +
> +               return 0;
> +       }
> +
> +       ctx->fallback = false;
> +
>         return qat_alg_skcipher_setkey(tfm, key, keylen,
>                                        ICP_QAT_HW_CIPHER_XTS_MODE);
>  }
> @@ -1066,9 +1081,19 @@ static int qat_alg_skcipher_blk_encrypt(struct skcipher_request *req)
>
>  static int qat_alg_skcipher_xts_encrypt(struct skcipher_request *req)
>  {
> +       struct crypto_skcipher *stfm = crypto_skcipher_reqtfm(req);
> +       struct qat_alg_skcipher_ctx *ctx = crypto_skcipher_ctx(stfm);
> +       struct skcipher_request *nreq = skcipher_request_ctx(req);
> +
>         if (req->cryptlen < XTS_BLOCK_SIZE)
>                 return -EINVAL;
>
> +       if (ctx->fallback) {
> +               memcpy(nreq, req, sizeof(*req));
> +               skcipher_request_set_tfm(nreq, ctx->ftfm);
> +               return crypto_skcipher_encrypt(nreq);
> +       }
> +
>         return qat_alg_skcipher_encrypt(req);
>  }
>
> @@ -1134,9 +1159,19 @@ static int qat_alg_skcipher_blk_decrypt(struct skcipher_request *req)
>
>  static int qat_alg_skcipher_xts_decrypt(struct skcipher_request *req)
>  {
> +       struct crypto_skcipher *stfm = crypto_skcipher_reqtfm(req);
> +       struct qat_alg_skcipher_ctx *ctx = crypto_skcipher_ctx(stfm);
> +       struct skcipher_request *nreq = skcipher_request_ctx(req);
> +
>         if (req->cryptlen < XTS_BLOCK_SIZE)
>                 return -EINVAL;
>
> +       if (ctx->fallback) {
> +               memcpy(nreq, req, sizeof(*req));
> +               skcipher_request_set_tfm(nreq, ctx->ftfm);
> +               return crypto_skcipher_decrypt(nreq);
> +       }
> +
>         return qat_alg_skcipher_decrypt(req);
>  }
>
> @@ -1200,6 +1235,23 @@ static int qat_alg_skcipher_init_tfm(struct crypto_skcipher *tfm)
>         return 0;
>  }
>
> +static int qat_alg_skcipher_init_xts_tfm(struct crypto_skcipher *tfm)
> +{
> +       struct qat_alg_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
> +       int reqsize;
> +
> +       ctx->ftfm = crypto_alloc_skcipher("xts(aes)", 0, CRYPTO_ALG_ASYNC);

Why are you only permitting synchronous fallbacks? If the logic above
is sound, and copies the base.complete and base.data fields as well,
the fallback can complete asynchronously without problems.

Note that SIMD s/w implementations of XTS(AES) are asynchronous as
well, as they use the crypto_simd helper which queues requests for
asynchronous completion if the context from which the request was
issued does not permit access to the SIMD register file (e.g., softirq
context on some architectures, if the interrupted context is also
using SIMD)


> +       if (IS_ERR(ctx->ftfm))
> +               return PTR_ERR(ctx->ftfm);
> +
> +       reqsize = max(sizeof(struct qat_crypto_request),
> +                     sizeof(struct skcipher_request) +
> +                     crypto_skcipher_reqsize(ctx->ftfm));
> +       crypto_skcipher_set_reqsize(tfm, reqsize);
> +
> +       return 0;
> +}
> +
>  static void qat_alg_skcipher_exit_tfm(struct crypto_skcipher *tfm)
>  {
>         struct qat_alg_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
> @@ -1227,6 +1279,15 @@ static void qat_alg_skcipher_exit_tfm(struct crypto_skcipher *tfm)
>         qat_crypto_put_instance(inst);
>  }
>
> +static void qat_alg_skcipher_exit_xts_tfm(struct crypto_skcipher *tfm)
> +{
> +       struct qat_alg_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
> +
> +       if (ctx->ftfm)
> +               crypto_free_skcipher(ctx->ftfm);
> +
> +       qat_alg_skcipher_exit_tfm(tfm);
> +}
>
>  static struct aead_alg qat_aeads[] = { {
>         .base = {
> @@ -1321,14 +1382,14 @@ static struct skcipher_alg qat_skciphers[] = { {
>         .base.cra_name = "xts(aes)",
>         .base.cra_driver_name = "qat_aes_xts",
>         .base.cra_priority = 4001,
> -       .base.cra_flags = CRYPTO_ALG_ASYNC,
> +       .base.cra_flags = CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK,
>         .base.cra_blocksize = AES_BLOCK_SIZE,
>         .base.cra_ctxsize = sizeof(struct qat_alg_skcipher_ctx),
>         .base.cra_alignmask = 0,
>         .base.cra_module = THIS_MODULE,
>
> -       .init = qat_alg_skcipher_init_tfm,
> -       .exit = qat_alg_skcipher_exit_tfm,
> +       .init = qat_alg_skcipher_init_xts_tfm,
> +       .exit = qat_alg_skcipher_exit_xts_tfm,
>         .setkey = qat_alg_skcipher_xts_setkey,
>         .decrypt = qat_alg_skcipher_xts_decrypt,
>         .encrypt = qat_alg_skcipher_xts_encrypt,
> --
> 2.26.2
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 4/4] crypto: qat - fallback for xts with 192 bit keys
  2020-06-26 18:15   ` Ard Biesheuvel
@ 2020-06-29 17:04     ` Giovanni Cabiddu
  2020-06-30 10:09       ` Ard Biesheuvel
  0 siblings, 1 reply; 8+ messages in thread
From: Giovanni Cabiddu @ 2020-06-29 17:04 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: Herbert Xu, Linux Crypto Mailing List, qat-linux

Thanks for your feedback Ard.

On Fri, Jun 26, 2020 at 08:15:16PM +0200, Ard Biesheuvel wrote:
> On Fri, 26 Jun 2020 at 10:04, Giovanni Cabiddu
> <giovanni.cabiddu@intel.com> wrote:
> >
> > +static int qat_alg_skcipher_init_xts_tfm(struct crypto_skcipher *tfm)
> > +{
> > +       struct qat_alg_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
> > +       int reqsize;
> > +
> > +       ctx->ftfm = crypto_alloc_skcipher("xts(aes)", 0, CRYPTO_ALG_ASYNC);
> 
> Why are you only permitting synchronous fallbacks? If the logic above
> is sound, and copies the base.complete and base.data fields as well,
> the fallback can complete asynchronously without problems.
> Note that SIMD s/w implementations of XTS(AES) are asynchronous as
> well, as they use the crypto_simd helper which queues requests for
> asynchronous completion if the context from which the request was
> issued does not permit access to the SIMD register file (e.g., softirq
> context on some architectures, if the interrupted context is also
> using SIMD)
I did it this way since I though I didn't have a way to test it with an
asynchronous sw implementation.
I changed this line to avoid masking the asynchronous implementations
and test it by forcing simd.c to use always cryptd (don't know if there
is a simpler way to do it).

Also, I added to the mask CRYPTO_ALG_NEED_FALLBACK so I don't get another
implementation that requires a fallback.

I'm going to send a v3.

Regards,

-- 
Giovanni

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 4/4] crypto: qat - fallback for xts with 192 bit keys
  2020-06-29 17:04     ` Giovanni Cabiddu
@ 2020-06-30 10:09       ` Ard Biesheuvel
  0 siblings, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2020-06-30 10:09 UTC (permalink / raw)
  To: Giovanni Cabiddu; +Cc: Herbert Xu, Linux Crypto Mailing List, qat-linux

On Mon, 29 Jun 2020 at 19:05, Giovanni Cabiddu
<giovanni.cabiddu@intel.com> wrote:
>
> Thanks for your feedback Ard.
>
> On Fri, Jun 26, 2020 at 08:15:16PM +0200, Ard Biesheuvel wrote:
> > On Fri, 26 Jun 2020 at 10:04, Giovanni Cabiddu
> > <giovanni.cabiddu@intel.com> wrote:
> > >
> > > +static int qat_alg_skcipher_init_xts_tfm(struct crypto_skcipher *tfm)
> > > +{
> > > +       struct qat_alg_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
> > > +       int reqsize;
> > > +
> > > +       ctx->ftfm = crypto_alloc_skcipher("xts(aes)", 0, CRYPTO_ALG_ASYNC);
> >
> > Why are you only permitting synchronous fallbacks? If the logic above
> > is sound, and copies the base.complete and base.data fields as well,
> > the fallback can complete asynchronously without problems.
> > Note that SIMD s/w implementations of XTS(AES) are asynchronous as
> > well, as they use the crypto_simd helper which queues requests for
> > asynchronous completion if the context from which the request was
> > issued does not permit access to the SIMD register file (e.g., softirq
> > context on some architectures, if the interrupted context is also
> > using SIMD)
> I did it this way since I though I didn't have a way to test it with an
> asynchronous sw implementation.
> I changed this line to avoid masking the asynchronous implementations
> and test it by forcing simd.c to use always cryptd (don't know if there
> is a simpler way to do it).
>

This is exactly how I tested it in the past, but note that the
extended testing that Eric implemented will also run from a context
where SIMD is disabled artificially, and so you should be getting this
behavior in any case.

> Also, I added to the mask CRYPTO_ALG_NEED_FALLBACK so I don't get another
> implementation that requires a fallback.
>
> I'm going to send a v3.
>
> Regards,
>
> --
> Giovanni

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-06-30 10:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-26  8:04 [PATCH v2 0/4] crypto: qat - fixes to aes xts Giovanni Cabiddu
2020-06-26  8:04 ` [PATCH v2 1/4] crypto: qat - allow xts requests not multiple of block Giovanni Cabiddu
2020-06-26  8:04 ` [PATCH v2 2/4] crypto: qat - validate xts key Giovanni Cabiddu
2020-06-26  8:04 ` [PATCH v2 3/4] crypto: qat - remove unused field in skcipher ctx Giovanni Cabiddu
2020-06-26  8:04 ` [PATCH v2 4/4] crypto: qat - fallback for xts with 192 bit keys Giovanni Cabiddu
2020-06-26 18:15   ` Ard Biesheuvel
2020-06-29 17:04     ` Giovanni Cabiddu
2020-06-30 10:09       ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).