[PATCH v6 0/9] Fixes and rework for paes_s390 and phmac_s390

[PATCH v6 0/9] Fixes and rework for paes_s390 and phmac_s390

[Harald Freudenberger]

Fix and rework some issues around arch/s390/paes_s390.c and
arch/s390/phmac_s390.c:

- Fix skcipher_walk return code handling in paes_s390
- Add scrub of some temp buffers
- Shift from using a mutex to using a semaphore in PAES CTR
  Surprisingly clang code analysis is able to deal with semaphores and
  thus the shift also fixes the issue with CONTEXT_ANALYSIS enabled.
- And some more fixes related to paes and phmac (see changelog).

For more details please see patch headers.

Changelog:

v1: initial version - however, all these patches are follow up patches
    from a similar patch queue for aes_s390.c
v2: - On one error path in the CTR implementation the scrubbing of a
      temp buffer could be bypassed. Also checked Sashikos claim about
      possible double free but this is not the case.
    - All paes algorithms did not set any base.cra_flags. So now set
      the ASYNC and the NO_FALLBACK flag.
    - If a request is pushed to the crypto engine there is another
      return code EBUSY also indicating success but the code handled
      this as a failure.
    - The very same was with the phmac implementation. So corrected
      the return code handling there as well.   
v3: - Reworked the -EBUSY handling in paes and phmac again. The
      synchronous path could have emitted -EBUSY also and would have
      triggered the failure handling. So map -EBUSY to -EINPROGRESS.
v4: - Mapping EBUSY to EINPROGRESS is not the right approach. So
      reworked again more thoroughly.
    - There came another reply from Sashiko about a possible double
      processing maybe even double free as a result of wrongly
      returning a failure code in the callback functions back to the
      engine when a request has been completed. The callback must
      return 0 - so another patch for this.
v5: - Sashiko found a potential for a deadlock with phmac where a
      persistent key conversion failure results in an -EBUSY which is
      as such written into the request. However, -EBUSY is swallowed
      by the engine layer and thus the calling process never gets it's
      callback invoked and may wait forever. So map -EBUSY to -EIO in
      the conversion function when there is a persistent inability to
      convert a key - in the end this is an IO failure. 
v6: - Added Fixes and Cc tags to patch #8.
    - And finally picked Heiko's update on arch/s390/Makefile to enable
      context analysis for the clang compiler.

Harald Freudenberger (8):
  s390/crypto: Fix return code handling at skcipher_walk_done in PAES
    algorithms
  s390/crypto: Fix missing scrub of temp buffers with PAES algorithm
  s390/crypto: Fix use of mutex in atomic context in PAES
  s390/crypto: Fix missing cra_flags in paes_s390
  s390/crypto: Fix handling of EBUSY in PAES when req is pushed to
    crypto engine
  s390/crypto: Fix handling of EBUSY in PHMAC when req is pushed to
    crypto engine
  s390/crypto: Fix wrong return code to engine in asynch callbacks
  s390/crypto: Map EBUSY to EIO when key conversion fails repeatedly

Heiko Carstens (1):
  s390/crypto: Enable CONTEXT_ANALYSIS

 arch/s390/crypto/Makefile     |   2 +
 arch/s390/crypto/paes_s390.c  | 100 +++++++++++++++++++++++-----------
 arch/s390/crypto/phmac_s390.c |  36 ++++++++----
 3 files changed, 97 insertions(+), 41 deletions(-)


base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
--
2.43.0

[PATCH v6 1/9] s390/crypto: Fix return code handling at skcipher_walk_done in PAES algorithms

[Harald Freudenberger]

All the 4 PAES cipher processing loops were not checking the return
value of skcipher_walk_done() immediately after calling it. This could
lead to error masking when both the walk operation failed and a
subsequent key conversion was needed (k < n condition).

Add immediate error checks after skcipher_walk_done() in all main
processing loops (ECB, CBC, CTR, XTS modes) to ensure walk errors are
properly propagated and not masked by subsequent operations.

With that comes a slight rework around the skcipher_walk_done()
invocation. It is now necessary to check if the walk has already been
finalized (walk->nbytes is then 0) or not to avoid double
de-allocation of resources held by the walk.

Fixes: 6cd87cb5ef6c ("s390/crypto: Rework protected key AES for true asynch support")
Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
Cc: stable@vger.kernel.org # 6.16+
---
 arch/s390/crypto/paes_s390.c | 38 +++++++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
index 8cfe6166c193..ccc7da106c3a 100644
--- a/arch/s390/crypto/paes_s390.c
+++ b/arch/s390/crypto/paes_s390.c
@@ -432,8 +432,11 @@ static int ecb_paes_do_crypt(struct s390_paes_ctx *ctx,
 		n = nbytes & ~(AES_BLOCK_SIZE - 1);
 		k = cpacf_km(ctx->fc | req_ctx->modifier, param,
 			     walk->dst.virt.addr, walk->src.virt.addr, n);
-		if (k)
+		if (k) {
 			rc = skcipher_walk_done(walk, nbytes - k);
+			if (rc)
+				goto out;
+		}
 		if (k < n) {
 			if (!maysleep) {
 				rc = -EKEYEXPIRED;
@@ -495,7 +498,7 @@ static int ecb_paes_crypt(struct skcipher_request *req, unsigned long modifier)
 			atomic_dec(&ctx->via_engine_ctr);
 	}
 
-	if (rc != -EINPROGRESS)
+	if (rc != -EINPROGRESS && walk->nbytes)
 		skcipher_walk_done(walk, rc);
 
 out:
@@ -558,7 +561,7 @@ static int ecb_paes_do_one_request(struct crypto_engine *engine, void *areq)
 		cond_resched();
 		pr_debug("rescheduling request\n");
 		return -ENOSPC;
-	} else if (rc) {
+	} else if (rc && walk->nbytes) {
 		skcipher_walk_done(walk, rc);
 	}
 
@@ -699,6 +702,8 @@ static int cbc_paes_do_crypt(struct s390_paes_ctx *ctx,
 		if (k) {
 			memcpy(walk->iv, param->iv, AES_BLOCK_SIZE);
 			rc = skcipher_walk_done(walk, nbytes - k);
+			if (rc)
+				goto out;
 		}
 		if (k < n) {
 			if (!maysleep) {
@@ -761,7 +766,7 @@ static int cbc_paes_crypt(struct skcipher_request *req, unsigned long modifier)
 			atomic_dec(&ctx->via_engine_ctr);
 	}
 
-	if (rc != -EINPROGRESS)
+	if (rc != -EINPROGRESS && walk->nbytes)
 		skcipher_walk_done(walk, rc);
 
 out:
@@ -824,7 +829,7 @@ static int cbc_paes_do_one_request(struct crypto_engine *engine, void *areq)
 		cond_resched();
 		pr_debug("rescheduling request\n");
 		return -ENOSPC;
-	} else if (rc) {
+	} else if (rc && walk->nbytes) {
 		skcipher_walk_done(walk, rc);
 	}
 
@@ -986,6 +991,11 @@ static int ctr_paes_do_crypt(struct s390_paes_ctx *ctx,
 				       AES_BLOCK_SIZE);
 			crypto_inc(walk->iv, AES_BLOCK_SIZE);
 			rc = skcipher_walk_done(walk, nbytes - k);
+			if (rc) {
+				if (locked)
+					mutex_unlock(&ctrblk_lock);
+				goto out;
+			}
 		}
 		if (k < n) {
 			if (!maysleep) {
@@ -1079,7 +1089,7 @@ static int ctr_paes_crypt(struct skcipher_request *req)
 			atomic_dec(&ctx->via_engine_ctr);
 	}
 
-	if (rc != -EINPROGRESS)
+	if (rc != -EINPROGRESS && walk->nbytes)
 		skcipher_walk_done(walk, rc);
 
 out:
@@ -1132,7 +1142,7 @@ static int ctr_paes_do_one_request(struct crypto_engine *engine, void *areq)
 		cond_resched();
 		pr_debug("rescheduling request\n");
 		return -ENOSPC;
-	} else if (rc) {
+	} else if (rc && walk->nbytes) {
 		skcipher_walk_done(walk, rc);
 	}
 
@@ -1310,8 +1320,11 @@ static int xts_paes_do_crypt_fullkey(struct s390_pxts_ctx *ctx,
 		n = nbytes & ~(AES_BLOCK_SIZE - 1);
 		k = cpacf_km(ctx->fc | req_ctx->modifier, param->key + offset,
 			     walk->dst.virt.addr, walk->src.virt.addr, n);
-		if (k)
+		if (k) {
 			rc = skcipher_walk_done(walk, nbytes - k);
+			if (rc)
+				goto out;
+		}
 		if (k < n) {
 			if (!maysleep) {
 				rc = -EKEYEXPIRED;
@@ -1404,8 +1417,11 @@ static int xts_paes_do_crypt_2keys(struct s390_pxts_ctx *ctx,
 		n = nbytes & ~(AES_BLOCK_SIZE - 1);
 		k = cpacf_km(ctx->fc | req_ctx->modifier, param->key + offset,
 			     walk->dst.virt.addr, walk->src.virt.addr, n);
-		if (k)
+		if (k) {
 			rc = skcipher_walk_done(walk, nbytes - k);
+			if (rc)
+				goto out;
+		}
 		if (k < n) {
 			if (!maysleep) {
 				rc = -EKEYEXPIRED;
@@ -1512,7 +1528,7 @@ static inline int xts_paes_crypt(struct skcipher_request *req, unsigned long mod
 			atomic_dec(&ctx->via_engine_ctr);
 	}
 
-	if (rc != -EINPROGRESS)
+	if (rc != -EINPROGRESS && walk->nbytes)
 		skcipher_walk_done(walk, rc);
 
 out:
@@ -1575,7 +1591,7 @@ static int xts_paes_do_one_request(struct crypto_engine *engine, void *areq)
 		cond_resched();
 		pr_debug("rescheduling request\n");
 		return -ENOSPC;
-	} else if (rc) {
+	} else if (rc && walk->nbytes) {
 		skcipher_walk_done(walk, rc);
 	}
 
-- 
2.43.0

[PATCH v6 2/9] s390/crypto: Fix missing scrub of temp buffers with PAES algorithm

[Harald Freudenberger]

In function ctr_paes_do_crypt() there is a buffer used to process
remaining bytes < AES_BLOCK_SIZE. This buffer was not scrubbed and
thus could lead to expose of unwanted data. Rework the code to
explicitly scrub the buffer at the end of the function to avoid
exposure of maybe sensitive data.

In function __xts_2keys_prep_param() change the existing scrub to
clean the whole param block instead of just the key field.

Fixes: 6cd87cb5ef6c ("s390/crypto: Rework protected key AES for true asynch support")
Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
Cc: stable@vger.kernel.org # 6.16+
---
 arch/s390/crypto/paes_s390.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
index ccc7da106c3a..d075b0241f1f 100644
--- a/arch/s390/crypto/paes_s390.c
+++ b/arch/s390/crypto/paes_s390.c
@@ -1044,6 +1044,7 @@ static int ctr_paes_do_crypt(struct s390_paes_ctx *ctx,
 	}
 
 out:
+	memzero_explicit(buf, sizeof(buf));
 	pr_debug("rc=%d\n", rc);
 	return rc;
 }
@@ -1377,7 +1378,7 @@ static inline int __xts_2keys_prep_param(struct s390_pxts_ctx *ctx,
 		memcpy(param->init, pcc_param.xts, 16);
 	}
 
-	memzero_explicit(pcc_param.key, sizeof(pcc_param.key));
+	memzero_explicit(&pcc_param, sizeof(pcc_param));
 	return rc;
 }
 
-- 
2.43.0

[PATCH v6 3/9] s390/crypto: Fix use of mutex in atomic context in PAES

[Harald Freudenberger]

The PAES CTR implementation used a mutex to lock one page of exclusive
memory for fast CTR processing. Unfortunately a mutex is not save to
use in atomic or interrupt context. So use a binary semaphore instead
which is save to use in such environments.

Fixes: 6cd87cb5ef6c ("s390/crypto: Rework protected key AES for true asynch support")
Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
Cc: stable@vger.kernel.org # 6.16+
---
 arch/s390/crypto/paes_s390.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
index d075b0241f1f..e8bbfc376ebb 100644
--- a/arch/s390/crypto/paes_s390.c
+++ b/arch/s390/crypto/paes_s390.c
@@ -19,7 +19,7 @@
 #include <linux/init.h>
 #include <linux/miscdevice.h>
 #include <linux/module.h>
-#include <linux/mutex.h>
+#include <linux/semaphore.h>
 #include <linux/spinlock.h>
 #include <crypto/aes.h>
 #include <crypto/algapi.h>
@@ -45,7 +45,7 @@ module_param_named(clrkey, pkey_clrkey_allowed, bool, 0444);
 MODULE_PARM_DESC(clrkey, "Allow clear key material (default N)");
 
 static u8 *ctrblk;
-static DEFINE_MUTEX(ctrblk_lock);
+static DEFINE_SEMAPHORE(ctrblk_sem, 1);
 
 static cpacf_mask_t km_functions, kmc_functions, kmctr_functions;
 
@@ -945,7 +945,8 @@ static int ctr_paes_do_crypt(struct s390_paes_ctx *ctx,
 	struct skcipher_walk *walk = &req_ctx->walk;
 	u8 buf[AES_BLOCK_SIZE], *ctrptr;
 	unsigned int nbytes, n, k;
-	int pk_state, locked, rc = 0;
+	int pk_state, rc = 0;
+	bool locked;
 
 	if (!req_ctx->param_init_done) {
 		/* fetch and check protected key state */
@@ -971,7 +972,7 @@ static int ctr_paes_do_crypt(struct s390_paes_ctx *ctx,
 	if (rc)
 		goto out;
 
-	locked = mutex_trylock(&ctrblk_lock);
+	locked = down_trylock(&ctrblk_sem) == 0;
 
 	/*
 	 * Note that in case of partial processing or failure the walk
@@ -993,21 +994,21 @@ static int ctr_paes_do_crypt(struct s390_paes_ctx *ctx,
 			rc = skcipher_walk_done(walk, nbytes - k);
 			if (rc) {
 				if (locked)
-					mutex_unlock(&ctrblk_lock);
+					up(&ctrblk_sem);
 				goto out;
 			}
 		}
 		if (k < n) {
 			if (!maysleep) {
 				if (locked)
-					mutex_unlock(&ctrblk_lock);
+					up(&ctrblk_sem);
 				rc = -EKEYEXPIRED;
 				goto out;
 			}
 			rc = paes_convert_key(ctx, tested);
 			if (rc) {
 				if (locked)
-					mutex_unlock(&ctrblk_lock);
+					up(&ctrblk_sem);
 				goto out;
 			}
 			spin_lock_bh(&ctx->pk_lock);
@@ -1016,7 +1017,7 @@ static int ctr_paes_do_crypt(struct s390_paes_ctx *ctx,
 		}
 	}
 	if (locked)
-		mutex_unlock(&ctrblk_lock);
+		up(&ctrblk_sem);
 
 	/* final block may be < AES_BLOCK_SIZE, copy only nbytes */
 	if (nbytes) {
-- 
2.43.0

[PATCH v6 4/9] s390/crypto: Fix missing cra_flags in paes_s390

[Harald Freudenberger]

The 4 algorithms implemented in paes_s390 never had any cra_flags
set. So add code which sets the cra_flag to CRYPTO_ALG_ASYNC and
CRYPTO_ALG_NO_FALLBACK.

Fixes: 4ccd065a69df ("crypto: ahash - Add support for drivers with no fallback")
Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
Cc: stable@vger.kernel.org # 6.17+
---
 arch/s390/crypto/paes_s390.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
index e8bbfc376ebb..03446bfac37f 100644
--- a/arch/s390/crypto/paes_s390.c
+++ b/arch/s390/crypto/paes_s390.c
@@ -579,6 +579,7 @@ static struct skcipher_engine_alg ecb_paes_alg = {
 		.base.cra_name	      = "ecb(paes)",
 		.base.cra_driver_name = "ecb-paes-s390",
 		.base.cra_priority    = 401,	/* combo: aes + ecb + 1 */
+		.base.cra_flags	      = CRYPTO_ALG_ASYNC | CRYPTO_ALG_NO_FALLBACK,
 		.base.cra_blocksize   = AES_BLOCK_SIZE,
 		.base.cra_ctxsize     = sizeof(struct s390_paes_ctx),
 		.base.cra_module      = THIS_MODULE,
@@ -847,6 +848,7 @@ static struct skcipher_engine_alg cbc_paes_alg = {
 		.base.cra_name	      = "cbc(paes)",
 		.base.cra_driver_name = "cbc-paes-s390",
 		.base.cra_priority    = 402,	/* cbc-paes-s390 + 1 */
+		.base.cra_flags	      = CRYPTO_ALG_ASYNC | CRYPTO_ALG_NO_FALLBACK,
 		.base.cra_blocksize   = AES_BLOCK_SIZE,
 		.base.cra_ctxsize     = sizeof(struct s390_paes_ctx),
 		.base.cra_module      = THIS_MODULE,
@@ -1162,6 +1164,7 @@ static struct skcipher_engine_alg ctr_paes_alg = {
 		.base.cra_name	      =	"ctr(paes)",
 		.base.cra_driver_name =	"ctr-paes-s390",
 		.base.cra_priority    =	402,	/* ecb-paes-s390 + 1 */
+		.base.cra_flags	      = CRYPTO_ALG_ASYNC | CRYPTO_ALG_NO_FALLBACK,
 		.base.cra_blocksize   =	1,
 		.base.cra_ctxsize     =	sizeof(struct s390_paes_ctx),
 		.base.cra_module      =	THIS_MODULE,
@@ -1611,6 +1614,7 @@ static struct skcipher_engine_alg xts_paes_alg = {
 		.base.cra_name	      =	"xts(paes)",
 		.base.cra_driver_name =	"xts-paes-s390",
 		.base.cra_priority    =	402,	/* ecb-paes-s390 + 1 */
+		.base.cra_flags	      = CRYPTO_ALG_ASYNC | CRYPTO_ALG_NO_FALLBACK,
 		.base.cra_blocksize   =	AES_BLOCK_SIZE,
 		.base.cra_ctxsize     =	sizeof(struct s390_pxts_ctx),
 		.base.cra_module      =	THIS_MODULE,
-- 
2.43.0

[PATCH v6 5/9] s390/crypto: Fix handling of EBUSY in PAES when req is pushed to crypto engine

[Harald Freudenberger]

When a request is transferred to the engine via
crypto_transfer_skcipher_request_to_engine() there are two return
codes signaling a successful transfer: EINPROGRESS and EBUSY. However
the correct handling of EBUSY was missing and has been added as a
return code indicating a successful transfer to the crypto engine.

Fixes: 6cd87cb5ef6c ("s390/crypto: Rework protected key AES for true asynch support")
Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
Cc: stable@vger.kernel.org # 6.16+
---
 arch/s390/crypto/paes_s390.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
index 03446bfac37f..2048327c1ffe 100644
--- a/arch/s390/crypto/paes_s390.c
+++ b/arch/s390/crypto/paes_s390.c
@@ -463,6 +463,7 @@ static int ecb_paes_crypt(struct skcipher_request *req, unsigned long modifier)
 	struct s390_paes_ctx *ctx = crypto_skcipher_ctx(tfm);
 	struct skcipher_walk *walk = &req_ctx->walk;
 	bool tested = crypto_skcipher_tested(tfm);
+	bool cleanup = true;
 	int rc;
 
 	/*
@@ -494,15 +495,17 @@ static int ecb_paes_crypt(struct skcipher_request *req, unsigned long modifier)
 	if (rc == 0 || rc == -EKEYEXPIRED) {
 		atomic_inc(&ctx->via_engine_ctr);
 		rc = crypto_transfer_skcipher_request_to_engine(paes_crypto_engine, req);
-		if (rc != -EINPROGRESS)
+		if (rc == -EINPROGRESS || rc == -EBUSY)
+			cleanup = false;
+		else
 			atomic_dec(&ctx->via_engine_ctr);
 	}
 
-	if (rc != -EINPROGRESS && walk->nbytes)
+	if (cleanup && walk->nbytes)
 		skcipher_walk_done(walk, rc);
 
 out:
-	if (rc != -EINPROGRESS)
+	if (cleanup)
 		memzero_explicit(&req_ctx->param, sizeof(req_ctx->param));
 	pr_debug("rc=%d\n", rc);
 	return rc;
@@ -732,6 +735,7 @@ static int cbc_paes_crypt(struct skcipher_request *req, unsigned long modifier)
 	struct s390_paes_ctx *ctx = crypto_skcipher_ctx(tfm);
 	struct skcipher_walk *walk = &req_ctx->walk;
 	bool tested = crypto_skcipher_tested(tfm);
+	bool cleanup = true;
 	int rc;
 
 	/*
@@ -763,15 +767,17 @@ static int cbc_paes_crypt(struct skcipher_request *req, unsigned long modifier)
 	if (rc == 0 || rc == -EKEYEXPIRED) {
 		atomic_inc(&ctx->via_engine_ctr);
 		rc = crypto_transfer_skcipher_request_to_engine(paes_crypto_engine, req);
-		if (rc != -EINPROGRESS)
+		if (rc == -EINPROGRESS || rc == -EBUSY)
+			cleanup = false;
+		else
 			atomic_dec(&ctx->via_engine_ctr);
 	}
 
-	if (rc != -EINPROGRESS && walk->nbytes)
+	if (cleanup && walk->nbytes)
 		skcipher_walk_done(walk, rc);
 
 out:
-	if (rc != -EINPROGRESS)
+	if (cleanup)
 		memzero_explicit(&req_ctx->param, sizeof(req_ctx->param));
 	pr_debug("rc=%d\n", rc);
 	return rc;
@@ -1059,6 +1065,7 @@ static int ctr_paes_crypt(struct skcipher_request *req)
 	struct s390_paes_ctx *ctx = crypto_skcipher_ctx(tfm);
 	struct skcipher_walk *walk = &req_ctx->walk;
 	bool tested = crypto_skcipher_tested(tfm);
+	bool cleanup = true;
 	int rc;
 
 	/*
@@ -1089,15 +1096,17 @@ static int ctr_paes_crypt(struct skcipher_request *req)
 	if (rc == 0 || rc == -EKEYEXPIRED) {
 		atomic_inc(&ctx->via_engine_ctr);
 		rc = crypto_transfer_skcipher_request_to_engine(paes_crypto_engine, req);
-		if (rc != -EINPROGRESS)
+		if (rc == -EINPROGRESS || rc == -EBUSY)
+			cleanup = false;
+		else
 			atomic_dec(&ctx->via_engine_ctr);
 	}
 
-	if (rc != -EINPROGRESS && walk->nbytes)
+	if (cleanup && walk->nbytes)
 		skcipher_walk_done(walk, rc);
 
 out:
-	if (rc != -EINPROGRESS)
+	if (cleanup)
 		memzero_explicit(&req_ctx->param, sizeof(req_ctx->param));
 	pr_debug("rc=%d\n", rc);
 	return rc;
@@ -1498,6 +1507,7 @@ static inline int xts_paes_crypt(struct skcipher_request *req, unsigned long mod
 	struct s390_pxts_ctx *ctx = crypto_skcipher_ctx(tfm);
 	struct skcipher_walk *walk = &req_ctx->walk;
 	bool tested = crypto_skcipher_tested(tfm);
+	bool cleanup = true;
 	int rc;
 
 	/*
@@ -1529,15 +1539,17 @@ static inline int xts_paes_crypt(struct skcipher_request *req, unsigned long mod
 	if (rc == 0 || rc == -EKEYEXPIRED) {
 		atomic_inc(&ctx->via_engine_ctr);
 		rc = crypto_transfer_skcipher_request_to_engine(paes_crypto_engine, req);
-		if (rc != -EINPROGRESS)
+		if (rc == -EINPROGRESS || rc == -EBUSY)
+			cleanup = false;
+		else
 			atomic_dec(&ctx->via_engine_ctr);
 	}
 
-	if (rc != -EINPROGRESS && walk->nbytes)
+	if (cleanup && walk->nbytes)
 		skcipher_walk_done(walk, rc);
 
 out:
-	if (rc != -EINPROGRESS)
+	if (cleanup)
 		memzero_explicit(&req_ctx->param, sizeof(req_ctx->param));
 	pr_debug("rc=%d\n", rc);
 	return rc;
-- 
2.43.0

[PATCH v6 6/9] s390/crypto: Fix handling of EBUSY in PHMAC when req is pushed to crypto engine

[Harald Freudenberger]

When a request is transferred to the engine via
crypto_transfer_hash_request_to_engine() there are two return codes
signaling a successful transfer: EINPROGRESS and EBUSY. However the
correct handling of EBUSY was missing and has been added as a return
code indicating a successful transfer to the crypto engine.

Fixes: cbbc675506cc ("crypto: s390 - New s390 specific protected key hash phmac")
Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
Cc: stable@vger.kernel.org # 6.17+
---
 arch/s390/crypto/phmac_s390.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/arch/s390/crypto/phmac_s390.c b/arch/s390/crypto/phmac_s390.c
index 03ca33ffe6cc..30c8a59af1a4 100644
--- a/arch/s390/crypto/phmac_s390.c
+++ b/arch/s390/crypto/phmac_s390.c
@@ -62,8 +62,10 @@ static inline int hwh_prepare(struct ahash_request *req,
  */
 static inline int hwh_advance(struct hash_walk_helper *hwh, int n)
 {
-	if (n < 0)
+	if (n < 0) {
+		hwh->walkbytes = n;
 		return crypto_hash_walk_done(&hwh->walk, n);
+	}
 
 	hwh->walkbytes -= n;
 	hwh->walkaddr += n;
@@ -606,6 +608,7 @@ static int phmac_update(struct ahash_request *req)
 	struct phmac_tfm_ctx *tfm_ctx = crypto_ahash_ctx(tfm);
 	struct kmac_sha2_ctx *kmac_ctx = &req_ctx->kmac_ctx;
 	struct hash_walk_helper *hwh = &req_ctx->hwh;
+	bool cleanup = true;
 	int rc;
 
 	/* prep the walk in the request context */
@@ -629,12 +632,15 @@ static int phmac_update(struct ahash_request *req)
 		req_ctx->async_op = OP_UPDATE;
 		atomic_inc(&tfm_ctx->via_engine_ctr);
 		rc = crypto_transfer_hash_request_to_engine(phmac_crypto_engine, req);
-		if (rc != -EINPROGRESS)
+		if (rc == -EINPROGRESS || rc == -EBUSY)
+			cleanup = false;
+		else
 			atomic_dec(&tfm_ctx->via_engine_ctr);
 	}
 
-	if (rc != -EINPROGRESS) {
-		hwh_advance(hwh, rc);
+	if (cleanup) {
+		if (hwh->walkbytes > 0)
+			hwh_advance(hwh, rc);
 		memzero_explicit(kmac_ctx, sizeof(*kmac_ctx));
 	}
 
@@ -649,6 +655,7 @@ static int phmac_final(struct ahash_request *req)
 	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
 	struct phmac_tfm_ctx *tfm_ctx = crypto_ahash_ctx(tfm);
 	struct kmac_sha2_ctx *kmac_ctx = &req_ctx->kmac_ctx;
+	bool cleanup = true;
 	int rc = 0;
 
 	/* Try synchronous operation if no active engine usage */
@@ -667,12 +674,14 @@ static int phmac_final(struct ahash_request *req)
 		req_ctx->async_op = OP_FINAL;
 		atomic_inc(&tfm_ctx->via_engine_ctr);
 		rc = crypto_transfer_hash_request_to_engine(phmac_crypto_engine, req);
-		if (rc != -EINPROGRESS)
+		if (rc == -EINPROGRESS || rc == -EBUSY)
+			cleanup = false;
+		else
 			atomic_dec(&tfm_ctx->via_engine_ctr);
 	}
 
 out:
-	if (rc != -EINPROGRESS)
+	if (cleanup)
 		memzero_explicit(kmac_ctx, sizeof(*kmac_ctx));
 	pr_debug("rc=%d\n", rc);
 	return rc;
@@ -685,6 +694,7 @@ static int phmac_finup(struct ahash_request *req)
 	struct phmac_tfm_ctx *tfm_ctx = crypto_ahash_ctx(tfm);
 	struct kmac_sha2_ctx *kmac_ctx = &req_ctx->kmac_ctx;
 	struct hash_walk_helper *hwh = &req_ctx->hwh;
+	bool cleanup = true;
 	int rc;
 
 	/* prep the walk in the request context */
@@ -716,15 +726,17 @@ static int phmac_finup(struct ahash_request *req)
 		/* req->async_op has been set to either OP_FINUP or OP_FINAL */
 		atomic_inc(&tfm_ctx->via_engine_ctr);
 		rc = crypto_transfer_hash_request_to_engine(phmac_crypto_engine, req);
-		if (rc != -EINPROGRESS)
+		if (rc == -EINPROGRESS || rc == -EBUSY)
+			cleanup = false;
+		else
 			atomic_dec(&tfm_ctx->via_engine_ctr);
 	}
 
-	if (rc != -EINPROGRESS)
+	if (cleanup && hwh->walkbytes > 0)
 		hwh_advance(hwh, rc);
 
 out:
-	if (rc != -EINPROGRESS)
+	if (cleanup)
 		memzero_explicit(kmac_ctx, sizeof(*kmac_ctx));
 	pr_debug("rc=%d\n", rc);
 	return rc;
-- 
2.43.0

[PATCH v6 7/9] s390/crypto: Fix wrong return code to engine in asynch callbacks

[Harald Freudenberger]

When crypto_finalize_hash_request() or
crypto_finalize_skcipher_request() explicitly completes a request, the
do_one_request callback must return 0 to indicate successful
handling. Returning a negative error code causes the crypto engine to
assume the driver failed to take ownership and triggers a second
completion via crypto_request_complete(), resulting in a double
completion. This pattern occurs in paes_s390.c 4 times and once in
phmac_s390.c.

Fixed in phmac_do_one_request() and all four paes do_one_request
callbacks (ecb, cbc, ctr, xts) by returning 0 after explicit
finalization instead of propagating the error code.

Fixes: 6cd87cb5ef6c ("s390/crypto: Rework protected key AES for true asynch support")
Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
Cc: stable@vger.kernel.org # 6.16+
---
 arch/s390/crypto/paes_s390.c  | 8 ++++----
 arch/s390/crypto/phmac_s390.c | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
index 2048327c1ffe..0bc360c7ef15 100644
--- a/arch/s390/crypto/paes_s390.c
+++ b/arch/s390/crypto/paes_s390.c
@@ -574,7 +574,7 @@ static int ecb_paes_do_one_request(struct crypto_engine *engine, void *areq)
 	atomic_dec(&ctx->via_engine_ctr);
 	crypto_finalize_skcipher_request(engine, req, rc);
 	local_bh_enable();
-	return rc;
+	return 0;
 }
 
 static struct skcipher_engine_alg ecb_paes_alg = {
@@ -846,7 +846,7 @@ static int cbc_paes_do_one_request(struct crypto_engine *engine, void *areq)
 	atomic_dec(&ctx->via_engine_ctr);
 	crypto_finalize_skcipher_request(engine, req, rc);
 	local_bh_enable();
-	return rc;
+	return 0;
 }
 
 static struct skcipher_engine_alg cbc_paes_alg = {
@@ -1165,7 +1165,7 @@ static int ctr_paes_do_one_request(struct crypto_engine *engine, void *areq)
 	atomic_dec(&ctx->via_engine_ctr);
 	crypto_finalize_skcipher_request(engine, req, rc);
 	local_bh_enable();
-	return rc;
+	return 0;
 }
 
 static struct skcipher_engine_alg ctr_paes_alg = {
@@ -1618,7 +1618,7 @@ static int xts_paes_do_one_request(struct crypto_engine *engine, void *areq)
 	atomic_dec(&ctx->via_engine_ctr);
 	crypto_finalize_skcipher_request(engine, req, rc);
 	local_bh_enable();
-	return rc;
+	return 0;
 }
 
 static struct skcipher_engine_alg xts_paes_alg = {
diff --git a/arch/s390/crypto/phmac_s390.c b/arch/s390/crypto/phmac_s390.c
index 30c8a59af1a4..89c5fa3dced2 100644
--- a/arch/s390/crypto/phmac_s390.c
+++ b/arch/s390/crypto/phmac_s390.c
@@ -945,7 +945,7 @@ static int phmac_do_one_request(struct crypto_engine *engine, void *areq)
 	atomic_dec(&tfm_ctx->via_engine_ctr);
 	crypto_finalize_hash_request(engine, req, rc);
 	local_bh_enable();
-	return rc;
+	return 0;
 }
 
 #define S390_ASYNC_PHMAC_ALG(x)						\
-- 
2.43.0

[PATCH v6 8/9] s390/crypto: Map EBUSY to EIO when key conversion fails repeatedly

[Harald Freudenberger]

When hardware persistently returns -EBUSY after exhausting retries,
the error propagates to crypto_finalize_*_request(). The crypto API's
completion wrapper treats -EBUSY as a queueing status and swallows it,
preventing the completion callback from firing. This causes callers
using crypto_wait_req() to block indefinitely.

Translate persistent -EBUSY to -EIO after retry exhaustion to ensure
proper error propagation and callback invocation.

Fixes: 6cd87cb5ef6c ("s390/crypto: Rework protected key AES for true asynch support")
Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
Cc: stable@vger.kernel.org # 6.16+
---
 arch/s390/crypto/paes_s390.c  | 4 ++++
 arch/s390/crypto/phmac_s390.c | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
index 0bc360c7ef15..17ef788e3053 100644
--- a/arch/s390/crypto/paes_s390.c
+++ b/arch/s390/crypto/paes_s390.c
@@ -220,6 +220,10 @@ static inline int convert_key(const u8 *key, unsigned int keylen,
 				      xflags);
 	}
 
+	/* But finally map -EBUSY to -EIO to indicate an IO failure */
+	if (rc == -EBUSY)
+		rc = -EIO;
+
 out:
 	pr_debug("rc=%d\n", rc);
 	return rc;
diff --git a/arch/s390/crypto/phmac_s390.c b/arch/s390/crypto/phmac_s390.c
index 89c5fa3dced2..44817c3d25cf 100644
--- a/arch/s390/crypto/phmac_s390.c
+++ b/arch/s390/crypto/phmac_s390.c
@@ -341,6 +341,10 @@ static inline int convert_key(const u8 *key, unsigned int keylen,
 				      xflags);
 	}
 
+	/* But finally map -EBUSY to -EIO to indicate an IO failure */
+	if (rc == -EBUSY)
+		rc = -EIO;
+
 out:
 	pr_debug("rc=%d\n", rc);
 	return rc;
-- 
2.43.0

[PATCH v6 9/9] s390/crypto: Enable CONTEXT_ANALYSIS

[Harald Freudenberger]

From: Heiko Carstens <hca@linux.ibm.com>

Enable CONTEXT_ANALYSIS since s390's crypto code compiles now without
warnings.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
Reviewed-by: Harald Freudenberger <freude@linux.ibm.com>
---
 arch/s390/crypto/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/s390/crypto/Makefile b/arch/s390/crypto/Makefile
index 48aeb0c0ffbd..1d6420813935 100644
--- a/arch/s390/crypto/Makefile
+++ b/arch/s390/crypto/Makefile
@@ -3,6 +3,8 @@
 # Cryptographic API
 #
 
+CONTEXT_ANALYSIS := y
+
 obj-$(CONFIG_CRYPTO_AES_S390) += aes_s390.o
 obj-$(CONFIG_CRYPTO_PAES_S390) += paes_s390.o
 obj-$(CONFIG_S390_PRNG) += prng.o
-- 
2.43.0

Re: [PATCH v6 4/9] s390/crypto: Fix missing cra_flags in paes_s390

[sashiko-bot]

> The 4 algorithms implemented in paes_s390 never had any cra_flags
> set. So add code which sets the cra_flag to CRYPTO_ALG_ASYNC and
> CRYPTO_ALG_NO_FALLBACK.
> 
> Fixes: 4ccd065a69df ("crypto: ahash - Add support for drivers with no fallback")
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821084648.23222-1-freude@linux.ibm.com?part=4

Re: [PATCH v6 7/9] s390/crypto: Fix wrong return code to engine in asynch callbacks

[sashiko-bot]

> When crypto_finalize_hash_request() or
> crypto_finalize_skcipher_request() explicitly completes a request, the
> do_one_request callback must return 0 to indicate successful
> handling. Returning a negative error code causes the crypto engine to
> assume the driver failed to take ownership and triggers a second
> completion via crypto_request_complete(), resulting in a double
> completion. This pattern occurs in paes_s390.c 4 times and once in
> phmac_s390.c.
> 
> Fixed in phmac_do_one_request() and all four paes do_one_request
> callbacks (ecb, cbc, ctr, xts) by returning 0 after explicit
> finalization instead of propagating the error code.
> 
> Fixes: 6cd87cb5ef6c ("s390/crypto: Rework protected key AES for true asynch support")
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821084648.23222-1-freude@linux.ibm.com?part=7

Re: [PATCH v6 9/9] s390/crypto: Enable CONTEXT_ANALYSIS

[sashiko-bot]

> From: Heiko Carstens <hca@linux.ibm.com>
> 
> Enable CONTEXT_ANALYSIS since s390's crypto code compiles now without
> warnings.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821084648.23222-1-freude@linux.ibm.com?part=9

Re: [PATCH v6 3/9] s390/crypto: Fix use of mutex in atomic context in PAES

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The `aes_s390.c` module contains the exact same locking bug in atomic context that this patch fixes in `paes_s390.c`. The `ctr_aes_crypt()` function uses `mutex_trylock(&ctrblk_lock)` and subsequently calls `mutex_unlock(&ctrblk_lock)`. As the patch's commit message notes, mutexes are not safe to use in atomic or interrupt context. Synchronous skcipher algorithms like `ctr-aes-s390` can be invoked directly from `softirq` context (e.g., by the IPsec stack). Calling `mutex_unlock()` in `softirq` is a strict violation of kernel locking rules.
--

--- Patch [3]: [PATCH v6 3/9] s390/crypto: Fix use of mutex in atomic context in PAES ---
commit 425e8f1d3532a176433992d286f917e0fd14ff9e
Author: Harald Freudenberger <freude@linux.ibm.com>

s390/crypto: Fix use of mutex in atomic context in PAES

This commit fixes a locking issue in the PAES CTR implementation where a
mutex was used in atomic or interrupt context. It replaces the mutex with
a binary semaphore which is safe to use in such environments.

> diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
> --- a/arch/s390/crypto/paes_s390.c
> +++ b/arch/s390/crypto/paes_s390.c
[ ... ]
> @@ -45,7 +45,7 @@ module_param_named(clrkey, pkey_clrkey_allowed, bool, 0444);
>  MODULE_PARM_DESC(clrkey, "Allow clear key material (default N)");
>  
>  static u8 *ctrblk;
> -static DEFINE_MUTEX(ctrblk_lock);
> +static DEFINE_SEMAPHORE(ctrblk_sem, 1);

[Severity: High]
This isn't a bug introduced by this patch, but arch/s390/crypto/aes_s390.c
appears to have the exact same bug that this patch is fixing here.

Looking at ctr_aes_crypt() in aes_s390.c:

	locked = mutex_trylock(&ctrblk_lock);

	ret = skcipher_walk_virt(&walk, req, false);
	while ((nbytes = walk.nbytes) >= AES_BLOCK_SIZE) {
		...
	}
	if (locked)
		mutex_unlock(&ctrblk_lock);

Since synchronous skcipher algorithms like ctr-aes-s390 can be invoked from
softirq context (such as by the IPsec stack), would calling mutex_unlock()
here trigger a lockdep splat or crash, especially on PREEMPT_RT where
mutex_unlock() must be called from process context?

Does aes_s390.c also need to be converted to use a binary semaphore?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821084648.23222-1-freude@linux.ibm.com?part=3

Re: [PATCH v6 1/9] s390/crypto: Fix return code handling at skcipher_walk_done in PAES algorithms

[sashiko-bot]

> All the 4 PAES cipher processing loops were not checking the return
> value of skcipher_walk_done() immediately after calling it. This could
> lead to error masking when both the walk operation failed and a
> subsequent key conversion was needed (k < n condition).
> 
> Add immediate error checks after skcipher_walk_done() in all main
> processing loops (ECB, CBC, CTR, XTS modes) to ensure walk errors are
> properly propagated and not masked by subsequent operations.
> 
> With that comes a slight rework around the skcipher_walk_done()
> invocation. It is now necessary to check if the walk has already been
> finalized (walk->nbytes is then 0) or not to avoid double
> de-allocation of resources held by the walk.
> 
> Fixes: 6cd87cb5ef6c ("s390/crypto: Rework protected key AES for true asynch support")
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821084648.23222-1-freude@linux.ibm.com?part=1

Re: [PATCH v6 2/9] s390/crypto: Fix missing scrub of temp buffers with PAES algorithm

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing explicit scrub of local stack buffer `buf` in `ctr_aes_crypt()` (standard AES algorithm).
- [High] Missing explicit scrub of temp buffers in `gcm_sg_walk` structures and AES keys in `gcm_aes_crypt()`.
--

--- Patch [2]: [PATCH v6 2/9] s390/crypto: Fix missing scrub of temp buffers with PAES algorithm ---
commit 0f77835bffb943e6b87472b41b755be562bb23c4
Author: Harald Freudenberger <freude@linux.ibm.com>

s390/crypto: Fix missing scrub of temp buffers with PAES algorithm

This commit explicitly zeroizes temporary stack buffers used for partial
block processing in the paes algorithm. It also updates the scrubbing
logic in the xts parameter preparation function to clear the entire
parameter block rather than just the key field.

> diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
> index ccc7da106c3a0..d075b0241f1f5 100644
> --- a/arch/s390/crypto/paes_s390.c
> +++ b/arch/s390/crypto/paes_s390.c
> @@ -1044,6 +1044,7 @@ static int ctr_paes_do_crypt(struct s390_paes_ctx *ctx,
>  	}
>  
>  out:
> +	memzero_explicit(buf, sizeof(buf));

[Severity: High]
This isn't a bug introduced by this patch, but while you are fixing the missing
scrub of buf here, there appears to be an identical pre-existing issue in the
standard aes implementation.

In arch/s390/crypto/aes_s390.c:ctr_aes_crypt(), the local stack buffer buf
is populated with cryptographic data when processing a final partial block,
but it returns without scrubbing the buffer:

		ret = skcipher_walk_done(&walk, 0);
	}

	return ret;
}

Since this path is reachable by unprivileged users invoking the ctr(aes)
algorithm with unaligned lengths, does ctr_aes_crypt() need a similar
memzero_explicit() call before returning to avoid leaving sensitive data on
the kernel stack?

[Severity: High]
This is another pre-existing issue in the same subsystem, but looking at
gcm_aes_crypt() in arch/s390/crypto/aes_s390.c, there are error paths that
bypass the existing memzero_explicit() cleanup completely:

			return -ENOMEM;
		if (gcm_out_walk_done(&gw_out, n) != n)
			return -ENOMEM;

If these error paths are taken when processing fragmented input data, does this
leave the aes key and hash subkey on the kernel stack?

Additionally, the normal exit path cleans the param block but doesn't appear
to clean the buf fields in the gcm_sg_walk structures (gw_in and gw_out):

	memzero_explicit(&param, sizeof(param));
	return ret;
}

Should these temporary buffers and early return paths also be scrubbed?

>  	pr_debug("rc=%d\n", rc);
>  	return rc;
>  }
> @@ -1377,7 +1378,7 @@ static inline int __xts_2keys_prep_param(struct s390_pxts_ctx *ctx,
>  		memcpy(param->init, pcc_param.xts, 16);
>  	}
>  
> -	memzero_explicit(pcc_param.key, sizeof(pcc_param.key));
> +	memzero_explicit(&pcc_param, sizeof(pcc_param));
>  	return rc;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821084648.23222-1-freude@linux.ibm.com?part=2

Re: [PATCH v6 8/9] s390/crypto: Map EBUSY to EIO when key conversion fails repeatedly

[sashiko-bot]

> When hardware persistently returns -EBUSY after exhausting retries,
> the error propagates to crypto_finalize_*_request(). The crypto API's
> completion wrapper treats -EBUSY as a queueing status and swallows it,
> preventing the completion callback from firing. This causes callers
> using crypto_wait_req() to block indefinitely.
> 
> Translate persistent -EBUSY to -EIO after retry exhaustion to ensure
> proper error propagation and callback invocation.
> 
> Fixes: 6cd87cb5ef6c ("s390/crypto: Rework protected key AES for true asynch support")
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821084648.23222-1-freude@linux.ibm.com?part=8

Re: [PATCH v6 6/9] s390/crypto: Fix handling of EBUSY in PHMAC when req is pushed to crypto engine

[sashiko-bot]

> When a request is transferred to the engine via
> crypto_transfer_hash_request_to_engine() there are two return codes
> signaling a successful transfer: EINPROGRESS and EBUSY. However the
> correct handling of EBUSY was missing and has been added as a return
> code indicating a successful transfer to the crypto engine.
> 
> Fixes: cbbc675506cc ("crypto: s390 - New s390 specific protected key hash phmac")
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821084648.23222-1-freude@linux.ibm.com?part=6

Re: [PATCH v6 3/9] s390/crypto: Fix use of mutex in atomic context in PAES

[Harald Freudenberger]

On 2026-08-21 10:56, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The `aes_s390.c` module contains the exact same locking bug
> in atomic context that this patch fixes in `paes_s390.c`. The
> `ctr_aes_crypt()` function uses `mutex_trylock(&ctrblk_lock)` and
> subsequently calls `mutex_unlock(&ctrblk_lock)`. As the patch's commit
> message notes, mutexes are not safe to use in atomic or interrupt
> context. Synchronous skcipher algorithms like `ctr-aes-s390` can be
> invoked directly from `softirq` context (e.g., by the IPsec stack).
> Calling `mutex_unlock()` in `softirq` is a strict violation of kernel
> locking rules.
> --
> 
> --- Patch [3]: [PATCH v6 3/9] s390/crypto: Fix use of mutex in atomic
> context in PAES ---
> commit 425e8f1d3532a176433992d286f917e0fd14ff9e
> Author: Harald Freudenberger <freude@linux.ibm.com>
> 
> s390/crypto: Fix use of mutex in atomic context in PAES
> 
> This commit fixes a locking issue in the PAES CTR implementation where 
> a
> mutex was used in atomic or interrupt context. It replaces the mutex 
> with
> a binary semaphore which is safe to use in such environments.
> 
>> diff --git a/arch/s390/crypto/paes_s390.c 
>> b/arch/s390/crypto/paes_s390.c
>> --- a/arch/s390/crypto/paes_s390.c
>> +++ b/arch/s390/crypto/paes_s390.c
> [ ... ]
>> @@ -45,7 +45,7 @@ module_param_named(clrkey, pkey_clrkey_allowed, 
>> bool, 0444);
>>  MODULE_PARM_DESC(clrkey, "Allow clear key material (default N)");
>> 
>>  static u8 *ctrblk;
>> -static DEFINE_MUTEX(ctrblk_lock);
>> +static DEFINE_SEMAPHORE(ctrblk_sem, 1);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but 
> arch/s390/crypto/aes_s390.c
> appears to have the exact same bug that this patch is fixing here.
> 
> Looking at ctr_aes_crypt() in aes_s390.c:
> 
> 	locked = mutex_trylock(&ctrblk_lock);
> 
> 	ret = skcipher_walk_virt(&walk, req, false);
> 	while ((nbytes = walk.nbytes) >= AES_BLOCK_SIZE) {
> 		...
> 	}
> 	if (locked)
> 		mutex_unlock(&ctrblk_lock);
> 
> Since synchronous skcipher algorithms like ctr-aes-s390 can be invoked 
> from
> softirq context (such as by the IPsec stack), would calling 
> mutex_unlock()
> here trigger a lockdep splat or crash, especially on PREEMPT_RT where
> mutex_unlock() must be called from process context?
> 
> Does aes_s390.c also need to be converted to use a binary semaphore?

Allready addressed and fixed with the aes_s390 patch series.

Re: [PATCH v6 2/9] s390/crypto: Fix missing scrub of temp buffers with PAES algorithm

[Harald Freudenberger]

On 2026-08-21 10:58, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Missing explicit scrub of local stack buffer `buf` in
> `ctr_aes_crypt()` (standard AES algorithm).
> - [High] Missing explicit scrub of temp buffers in `gcm_sg_walk`
> structures and AES keys in `gcm_aes_crypt()`.
> --
> 
> --- Patch [2]: [PATCH v6 2/9] s390/crypto: Fix missing scrub of temp
> buffers with PAES algorithm ---
> commit 0f77835bffb943e6b87472b41b755be562bb23c4
> Author: Harald Freudenberger <freude@linux.ibm.com>
> 
> s390/crypto: Fix missing scrub of temp buffers with PAES algorithm
> 
> This commit explicitly zeroizes temporary stack buffers used for 
> partial
> block processing in the paes algorithm. It also updates the scrubbing
> logic in the xts parameter preparation function to clear the entire
> parameter block rather than just the key field.
> 
>> diff --git a/arch/s390/crypto/paes_s390.c 
>> b/arch/s390/crypto/paes_s390.c
>> index ccc7da106c3a0..d075b0241f1f5 100644
>> --- a/arch/s390/crypto/paes_s390.c
>> +++ b/arch/s390/crypto/paes_s390.c
>> @@ -1044,6 +1044,7 @@ static int ctr_paes_do_crypt(struct 
>> s390_paes_ctx *ctx,
>>  	}
>> 
>>  out:
>> +	memzero_explicit(buf, sizeof(buf));
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but while you are fixing the 
> missing
> scrub of buf here, there appears to be an identical pre-existing issue 
> in the
> standard aes implementation.
> 
> In arch/s390/crypto/aes_s390.c:ctr_aes_crypt(), the local stack buffer 
> buf
> is populated with cryptographic data when processing a final partial 
> block,
> but it returns without scrubbing the buffer:
> 
> 		ret = skcipher_walk_done(&walk, 0);
> 	}
> 
> 	return ret;
> }
> 
> Since this path is reachable by unprivileged users invoking the 
> ctr(aes)
> algorithm with unaligned lengths, does ctr_aes_crypt() need a similar
> memzero_explicit() call before returning to avoid leaving sensitive 
> data on
> the kernel stack?
> 
> [Severity: High]
> This is another pre-existing issue in the same subsystem, but looking 
> at
> gcm_aes_crypt() in arch/s390/crypto/aes_s390.c, there are error paths 
> that
> bypass the existing memzero_explicit() cleanup completely:
> 
> 			return -ENOMEM;
> 		if (gcm_out_walk_done(&gw_out, n) != n)
> 			return -ENOMEM;
> 
> If these error paths are taken when processing fragmented input data, 
> does this
> leave the aes key and hash subkey on the kernel stack?
> 
> Additionally, the normal exit path cleans the param block but doesn't 
> appear
> to clean the buf fields in the gcm_sg_walk structures (gw_in and 
> gw_out):
> 
> 	memzero_explicit(&param, sizeof(param));
> 	return ret;
> }
> 
> Should these temporary buffers and early return paths also be scrubbed?
> 
>>  	pr_debug("rc=%d\n", rc);
>>  	return rc;
>>  }
>> @@ -1377,7 +1378,7 @@ static inline int __xts_2keys_prep_param(struct 
>> s390_pxts_ctx *ctx,
>>  		memcpy(param->init, pcc_param.xts, 16);
>>  	}
>> 
>> -	memzero_explicit(pcc_param.key, sizeof(pcc_param.key));
>> +	memzero_explicit(&pcc_param, sizeof(pcc_param));
>>  	return rc;
>>  }

already addressed and fixed in the aes_s390 patch series.

Re: [PATCH v6 5/9] s390/crypto: Fix handling of EBUSY in PAES when req is pushed to crypto engine

[sashiko-bot]

> When a request is transferred to the engine via
> crypto_transfer_skcipher_request_to_engine() there are two return
> codes signaling a successful transfer: EINPROGRESS and EBUSY. However
> the correct handling of EBUSY was missing and has been added as a
> return code indicating a successful transfer to the crypto engine.
> 
> Fixes: 6cd87cb5ef6c ("s390/crypto: Rework protected key AES for true asynch support")
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821084648.23222-1-freude@linux.ibm.com?part=5