Skip to contentSkip to conversation

crypto - qce: fix self-test issues in aead

PATCH Superseded Bartosz Golaszewski 1 participant 4 messages Oct 19

Subsystems (4)

v11 of 12 Superseded Latest is v12 →

lore
Share & tools
lore
#0 lore raw

v11 cover 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 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 | 40 ++++++++++++++++++++++++++++++++++------
 1 file changed, 34 insertions(+), 6 deletions(-)

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

Best regards,

Signature · 1 line
Bartosz Golaszewski <[email protected]>
#1 lore raw

PATCH v11 1/3 crypto: qce - Don't modify the TFM context for per-request fallback

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.

Fixes b51dcf05c1e9 ("crypto: qce - Schedule fallback aead algorithm")
---
 drivers/crypto/qce/aead.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/crypto/qce/aead.c +3 -5
index e0061bfab216861444e36700d6530e7a3cd347e7..81c1a9598474c7c1623e71a9eac876fcc0c111b4 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);
Signature · 1 line
2.47.3
#2 lore raw

PATCH v11 2/3 crypto: qce - Use a fallback for CCM with a partial final block

In reply to Bartosz Golaszewski

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.

Fixes 9363efb4181c ("crypto: qce - Add support for AEAD algorithms")
---
 drivers/crypto/qce/aead.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/crypto/qce/aead.c +8 -0
index 81c1a9598474c7c1623e71a9eac876fcc0c111b4..f34e4feb3004d9bf63c2c8b2aeb3975d86ac486b 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);
Signature · 1 line
2.47.3
#3 lore raw

PATCH v11 3/3 crypto: qce - Use fallback for CCM with a fragmented payload

In reply to Bartosz Golaszewski

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.

Fixes 9363efb4181c ("crypto: qce - Add support for AEAD algorithms")
---
 drivers/crypto/qce/aead.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)
drivers/crypto/qce/aead.c +23 -1
index f34e4feb3004d9bf63c2c8b2aeb3975d86ac486b..ecc33570804fa435db9ec9fd6fbf38c3c8708e2e 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);
Signature · 1 line
2.47.3