crypto - qce: fix self-test issues in aead

As requested by Herbert: this is the aead part of the QCE fixes sent separately for easier review.

---
Changes in v12:
- clear the ctx->need_fallback flag for key sizes other than AES-192
(sashiko)
- Link to v11: https://patch.msgid.link/[email protected]

Changes in v11:
- Split out the aead patches into a smaller series for easier review
- add a patch fixing a race condition resulting from sharing
ctx->eed_fallback by concurrent requests
- Link to v10: https://patch.msgid.link/[email protected]

Changes in v10:
- don't pass a NULL buffer to sg_init_one()
- fix byte order of the imported hash state on empty finalization
- don't recompute the CTR counter in software for every request
- reject skcipher requests with a too short scatterlist
- check fragmentation for empty CCM decryption requests too
- don't use a static loop iterator (duh...)
- initialize the return value in qce_handle_queue()
- Link to v9: https://patch.msgid.link/[email protected]

Changes in v9:
- Rebase on top of cryptodev/master
- Rephrase Kconfig help text changes
- Link to v8: https://patch.msgid.link/[email protected]

Changes in v8:
- Add a patch making the crypto driver *not* register algos by default
but only if the user explicitly requests it with the provided module
parameter
- Use the "offloader" name for the crypto IP instead of "accelerator"
- Fix device module table issues
- Make the core driver use a list (despite it only having a single
member now) of child auxiliary devices to register to make it clear we
have the intention of extending this once we start the work on secure
media playback
- Link to v7: https://patch.msgid.link/[email protected]

Changes in v7:
- Add follow-up changes converting the QCE crypto driver to auxiliary
bus, adding the core QCE driver under drivers/soc/ registering the
auxiliary device and removing the BROKEN Kconfig label
- Link to v6: https://patch.msgid.link/[email protected]

Changes in v6:
- Handle all zero-length HMAC finalizations (like imported state with
data already hashed), not only the genuinely empty message case
- Add an additional patch addressing the fragmented skcipher payload
issue
- Link to v5: https://patch.msgid.link/[email protected]

Changes in v5:
- Dropped patch 1/8 that's already queued
- Use the pre-allocated fallback ahash for HMAC transforms (Herbert)
- Link to v4: https://patch.msgid.link/[email protected]

Changes in v4:
- Remove remaining ECB and DES3 bits
- Pick up tags
- Link to v3: https://patch.msgid.link/[email protected]

Changes in v3:
- Remove even more algorithms and dead code in patch 1/8
- Link to v2: https://patch.msgid.link/[email protected]

Changes in v2:
- Add fixes for the full suite of crypto self-tests
- Add Fixes and Cc tags
- Link to v1: https://patch.msgid.link/[email protected]/

---
Bartosz Golaszewski (3):
crypto: qce - Don't modify the TFM context for per-request fallback
crypto: qce - Use a fallback for CCM with a partial final block
crypto: qce - Use fallback for CCM with a fragmented payload

 drivers/crypto/qce/aead.c | 46 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 36 insertions(+), 10 deletions(-)

---
base-commit: 0aac16003b01b9282491ea3d454697c2dc7e8c78
change-id: 20261019-qce-fixes-aead-e229fa2015e3

Best regards,

Bartosz Golaszewski <[email protected]>

qce_aead_crypt() records a per-request decision to use the software fallback (on zero-length message) in ctx->need_fallback, which lives in the TFM context shared by all concurrent requests on the transform, and then clears it again. This has two problems:

1. Concurrent requests race on the flag: one request can observe and clear the flag set for another, so that a request that needs the fallback is sent to the engine while an unrelated one is diverted to the fallback.

2. ctx->need_fallback is also set by setkey() for AES-192 keys, which the engine cannot handle for any request. Clearing it after the first fallback request makes every following request on that TFM go to the hardware with an unsupported key size.

Use a local variable seeded from the per-TFM flag for the per-request decision and stop clearing the flag.

---
 drivers/crypto/qce/aead.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

index e0061bfab216861444e36700d6530e7a3cd347e7..b7873852249dffeb91d09df3bae6534f77c08b7e 100644
@@ -499,6 +499,7 @@ static int qce_aead_crypt(struct aead_request *req, int encrypt)
 	struct qce_aead_ctx *ctx = crypto_aead_ctx(tfm);
 	struct qce_alg_template *tmpl = to_aead_tmpl(tfm);
 	unsigned int blocksize = crypto_aead_blocksize(tfm);
+	bool fallback = ctx->need_fallback;
 
 	rctx->flags  = tmpl->alg_flags;
 	rctx->flags |= encrypt ? QCE_ENCRYPT : QCE_DECRYPT;

@@ -511,14 +512,11 @@ static int qce_aead_crypt(struct aead_request *req, int encrypt)
 	/* CE does not handle 0 length messages */
 	if (!rctx->cryptlen) {
 		if (!(IS_CCM(rctx->flags) && IS_DECRYPT(rctx->flags)))
-			ctx->need_fallback = true;
+			fallback = true;
 	}
 
 	/* If fallback is needed, schedule and exit */
-	if (ctx->need_fallback) {
-		/* Reset need_fallback in case the same ctx is used for another transaction */
-		ctx->need_fallback = false;
-
+	if (fallback) {
 		aead_request_set_tfm(&rctx->fallback_req, ctx->fallback);
 		aead_request_set_callback(&rctx->fallback_req, req->base.flags,
 					  req->base.complete, req->base.data);

@@ -577,8 +575,7 @@ static int qce_aead_ccm_setkey(struct crypto_aead *tfm, const u8 *key,
 	memcpy(ctx->enc_key, key, keylen);
 	memcpy(ctx->auth_key, key, keylen);
 
-	if (keylen == AES_KEYSIZE_192)
-		ctx->need_fallback = true;
+	ctx->need_fallback = (keylen == AES_KEYSIZE_192);
 
 	return IS_CCM_RFC4309(flags) ?
 		crypto_aead_setkey(ctx->fallback, key, keylen + QCE_CCM4309_SALT_SIZE) :

@@ -606,8 +603,7 @@ static int qce_aead_setkey(struct crypto_aead *tfm, const u8 *key, unsigned int
 		    authenc_keys.enckeylen != AES_KEYSIZE_192 &&
 		    authenc_keys.enckeylen != AES_KEYSIZE_256)
 			return -EINVAL;
-		if (authenc_keys.enckeylen == AES_KEYSIZE_192)
-			ctx->need_fallback = true;
+		ctx->need_fallback = (authenc_keys.enckeylen == AES_KEYSIZE_192);
 	}
 
 	ctx->enc_keylen = authenc_keys.enckeylen;

2.47.3

CCM builds on AES-CTR for encryption, and the crypto engine stalls on a partial final block just as it does for plain ctr(aes): a payload whose length is not a multiple of the AES block size leaves the operation incomplete and fails with a hardware operation error. This was caught by the ccm(aes) crypto self-tests.

Force the software fallback for CCM requests whose message length is not block aligned, reusing the driver's existing need_fallback mechanism.

---
 drivers/crypto/qce/aead.c | 8 ++++++++
 1 file changed, 8 insertions(+)

index b7873852249dffeb91d09df3bae6534f77c08b7e..19d7e92aaeda0df1072114fa0435dc40e1eb6849 100644
@@ -515,6 +515,14 @@ static int qce_aead_crypt(struct aead_request *req, int encrypt)
 			fallback = true;
 	}
 
+	/*
+	 * CCM uses AES-CTR internally and the CE stalls on a partial final
+	 * block, so a payload that is not a multiple of the block size has to
+	 * be handled by the fallback.
+	 */
+	if (IS_CCM(rctx->flags) && !IS_ALIGNED(rctx->cryptlen, AES_BLOCK_SIZE))
+		fallback = true;
+
 	/* If fallback is needed, schedule and exit */
 	if (fallback) {
 		aead_request_set_tfm(&rctx->fallback_req, ctx->fallback);

2.47.3

The crypto engine reliably processes CCM only when the message payload is a single contiguous buffer. The associated data is already linearized into a bounce buffer before being submitted, but when the payload itself is split across multiple scatterlist entries the engine stalls waiting for input and the request fails with a hardware operation error. This was uncovered by the crypto self-tests, which feed the algorithms randomly fragmented buffers.

Detect a payload that spans more than one scatterlist entry (in either the source or the destination, skipping past the associated data) and route the request to the software fallback.

---
 drivers/crypto/qce/aead.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

index 19d7e92aaeda0df1072114fa0435dc40e1eb6849..8ce1a9f3c9d80ba46d5c074cc3ef45c69acf52ce 100644
@@ -498,7 +498,8 @@ static int qce_aead_crypt(struct aead_request *req, int encrypt)
 	struct qce_aead_reqctx *rctx = aead_request_ctx_dma(req);
 	struct qce_aead_ctx *ctx = crypto_aead_ctx(tfm);
 	struct qce_alg_template *tmpl = to_aead_tmpl(tfm);
-	unsigned int blocksize = crypto_aead_blocksize(tfm);
+	unsigned int blocksize = crypto_aead_blocksize(tfm), authsize;
+	struct scatterlist __sg[2], *msg_sg;
 	bool fallback = ctx->need_fallback;
 
 	rctx->flags  = tmpl->alg_flags;

@@ -523,6 +524,27 @@ static int qce_aead_crypt(struct aead_request *req, int encrypt)
 	if (IS_CCM(rctx->flags) && !IS_ALIGNED(rctx->cryptlen, AES_BLOCK_SIZE))
 		fallback = true;
 
+	/*
+	 * The CE reliably processes CCM only when the message payload is a
+	 * single contiguous buffer. The associated data is linearized into a
+	 * bounce buffer before being handed to the engine, but a fragmented
+	 * payload makes the engine stall waiting for input, so route those
+	 * requests to the fallback.
+	 */
+	if (IS_CCM(rctx->flags)) {
+		authsize = ctx->authsize;
+
+		msg_sg = scatterwalk_ffwd(__sg, req->src, req->assoclen);
+		if (sg_nents_for_len(msg_sg, rctx->cryptlen +
+				     (encrypt ? 0 : authsize)) > 1)
+			fallback = true;
+
+		msg_sg = scatterwalk_ffwd(__sg, req->dst, req->assoclen);
+		if (sg_nents_for_len(msg_sg, rctx->cryptlen +
+				     (encrypt ? authsize : 0)) > 1)
+			fallback = true;
+	}
+
 	/* If fallback is needed, schedule and exit */
 	if (fallback) {
 		aead_request_set_tfm(&rctx->fallback_req, ctx->fallback);

2.47.3

Ok, looks like there are no more sashiko comments on this one as well. Could we get the three separate series with fixes upstream before v7.4-rc1 to enable further rework of the driver? Once in linux-next, I'll also resend the driver adding the new module parameter and removing the BROKEN label.

Bart

All applied. Thanks.

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