[PATCH 0/3] s390/crypto: Enable CONTEXT_ANALYSIS

[PATCH 0/3] s390/crypto: Enable CONTEXT_ANALYSIS

[Heiko Carstens]

Enable CONTEXT_ANALYSYS for s390's crypto code.

Static code checking for acquiring and releasing locks used to be done
with sparse. That was removed with [1] and replaced with a clang based
approach [2]. The new approach requires that each subsystem needs to be
explicitly enabled for checking.

Do that for s390's crypto code. In order to avoid false positives the
code has to be slightly reworked, since conditionally acquiring and
releasing locks does not work with the checker (besides that this is
sub optimal coding style).

[1] 5b63d0ae94cc ("compiler-context-analysis: Remove Sparse support")
[2] 3269701cb256 ("compiler-context-analysis: Add infrastructure for Context Analysis with Clang")

Note:

- "Pre-existing issues" reported by AI will not be addressed in the
  context of this series

Heiko Carstens (3):
  s390/crypto: Rework ctr_aes_crypt() to remove conditional locking
  s390/crypto: Rework ctr_paes_do_crypt() to remove conditional locking
  s390/crypto: Enable CONTEXT_ANALYSIS

 arch/s390/crypto/Makefile    |   2 +
 arch/s390/crypto/aes_s390.c  |  54 +++++++++++-------
 arch/s390/crypto/paes_s390.c | 104 ++++++++++++++++++++---------------
 3 files changed, 97 insertions(+), 63 deletions(-)

-- 
2.53.0

[PATCH 1/3] s390/crypto: Rework ctr_aes_crypt() to remove conditional locking

[Heiko Carstens]

Clang's compiler based static context analysis does not work with locks
that are conditionally taken like in ctr_aes_crypt():

arch/s390/crypto/aes_s390.c:585:13: warning: mutex 'ctrblk_lock' is not held on every path through here
      [-Wthread-safety-analysis]
  585 |                 ctrptr = (n > AES_BLOCK_SIZE) ? ctrblk : walk.iv;
      |                           ^

Given that code which takes locks conditionally can be considered
suboptimal rework ctr_aes_crypt() to get rid of this.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/crypto/aes_s390.c | 54 ++++++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 19 deletions(-)

diff --git a/arch/s390/crypto/aes_s390.c b/arch/s390/crypto/aes_s390.c
index 62edc66d5478..4b7e47423e89 100644
--- a/arch/s390/crypto/aes_s390.c
+++ b/arch/s390/crypto/aes_s390.c
@@ -562,37 +562,53 @@ static unsigned int __ctrblk_init(u8 *ctrptr, u8 *iv, unsigned int nbytes)
 	return n;
 }
 
+static int __ctr_aes_crypt(struct s390_aes_ctx *sctx,
+			   struct skcipher_walk *walk, bool locked)
+{
+	unsigned int n, nbytes;
+	int ret = 0;
+	u8 *ctrptr;
+
+	while ((nbytes = walk->nbytes) >= AES_BLOCK_SIZE) {
+		n = AES_BLOCK_SIZE;
+		if (nbytes >= 2 * AES_BLOCK_SIZE && locked)
+			n = __ctrblk_init(ctrblk, walk->iv, nbytes);
+		ctrptr = (n > AES_BLOCK_SIZE) ? ctrblk : walk->iv;
+		cpacf_kmctr(sctx->fc, sctx->key, walk->dst.virt.addr,
+			    walk->src.virt.addr, n, ctrptr);
+		if (ctrptr == ctrblk) {
+			memcpy(walk->iv, ctrptr + n - AES_BLOCK_SIZE,
+			       AES_BLOCK_SIZE);
+		}
+		crypto_inc(walk->iv, AES_BLOCK_SIZE);
+		ret = skcipher_walk_done(walk, nbytes - n);
+	}
+
+	return ret;
+}
+
 static int ctr_aes_crypt(struct skcipher_request *req)
 {
 	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
 	struct s390_aes_ctx *sctx = crypto_skcipher_ctx(tfm);
-	u8 buf[AES_BLOCK_SIZE], *ctrptr;
 	struct skcipher_walk walk;
-	unsigned int n, nbytes;
-	int ret, locked;
+	u8 buf[AES_BLOCK_SIZE];
+	unsigned int nbytes;
+	int ret;
 
 	if (unlikely(!sctx->fc))
 		return fallback_skcipher_crypt(sctx, req, 0);
 
-	locked = mutex_trylock(&ctrblk_lock);
-
 	ret = skcipher_walk_virt(&walk, req, false);
-	while ((nbytes = walk.nbytes) >= AES_BLOCK_SIZE) {
-		n = AES_BLOCK_SIZE;
 
-		if (nbytes >= 2*AES_BLOCK_SIZE && locked)
-			n = __ctrblk_init(ctrblk, walk.iv, nbytes);
-		ctrptr = (n > AES_BLOCK_SIZE) ? ctrblk : walk.iv;
-		cpacf_kmctr(sctx->fc, sctx->key, walk.dst.virt.addr,
-			    walk.src.virt.addr, n, ctrptr);
-		if (ctrptr == ctrblk)
-			memcpy(walk.iv, ctrptr + n - AES_BLOCK_SIZE,
-			       AES_BLOCK_SIZE);
-		crypto_inc(walk.iv, AES_BLOCK_SIZE);
-		ret = skcipher_walk_done(&walk, nbytes - n);
-	}
-	if (locked)
+	if (mutex_trylock(&ctrblk_lock)) {
+		ret = __ctr_aes_crypt(sctx, &walk, true);
 		mutex_unlock(&ctrblk_lock);
+	} else {
+		ret = __ctr_aes_crypt(sctx, &walk, false);
+	}
+
+	nbytes = walk.nbytes;
 	/*
 	 * final block may be < AES_BLOCK_SIZE, copy only nbytes
 	 */
-- 
2.53.0

Re: [PATCH 1/3] s390/crypto: Rework ctr_aes_crypt() to remove conditional locking

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The error code returned by `skcipher_walk_virt()` is unconditionally overwritten by the return value of `__ctr_aes_crypt()`, silently dropping errors.
--

--- Patch [1]: [PATCH 1/3] s390/crypto: Rework ctr_aes_crypt() to remove conditional locking ---
commit 83af1e74cacdeddf34088df594504204b40a3d99
Author: Heiko Carstens <hca@linux.ibm.com>

s390/crypto: Rework ctr_aes_crypt() to remove conditional locking

This commit removes conditional locking in ctr_aes_crypt() to address a Clang
thread-safety warning. The block processing loop is moved into a new helper
function __ctr_aes_crypt().

> diff --git a/arch/s390/crypto/aes_s390.c b/arch/s390/crypto/aes_s390.c
> index 62edc66d54788..4b7e47423e892 100644
> --- a/arch/s390/crypto/aes_s390.c
> +++ b/arch/s390/crypto/aes_s390.c

[ ... ]

> @@ -589,37 +604,53 @@ static int ctr_aes_crypt(struct skcipher_request *req)
>  {
>  	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
>  	struct s390_aes_ctx *sctx = crypto_skcipher_ctx(tfm);
> -	u8 buf[AES_BLOCK_SIZE], *ctrptr;
>  	struct skcipher_walk walk;
> -	unsigned int n, nbytes;
> -	int ret, locked;
> +	u8 buf[AES_BLOCK_SIZE];
> +	unsigned int nbytes;
> +	int ret;
>  
>  	if (unlikely(!sctx->fc))
>  		return fallback_skcipher_crypt(sctx, req, 0);
>  
> -	locked = mutex_trylock(&ctrblk_lock);
> -
>  	ret = skcipher_walk_virt(&walk, req, false);
> -	while ((nbytes = walk.nbytes) >= AES_BLOCK_SIZE) {
> -		n = AES_BLOCK_SIZE;

[ ... ]

> -		crypto_inc(walk.iv, AES_BLOCK_SIZE);
> -		ret = skcipher_walk_done(&walk, nbytes - n);
> -	}
> -	if (locked)
> +
> +	if (mutex_trylock(&ctrblk_lock)) {
> +		ret = __ctr_aes_crypt(sctx, &walk, true);
>  		mutex_unlock(&ctrblk_lock);
> +	} else {
> +		ret = __ctr_aes_crypt(sctx, &walk, false);
> +	}

[Severity: High]
Does this code accidentally drop the error code from skcipher_walk_virt()?

If skcipher_walk_virt() fails and returns a negative error code (e.g. -ENOMEM
or -EINVAL), ret is immediately overwritten by the return value of
__ctr_aes_crypt().

Because walk.nbytes is 0 on error, __ctr_aes_crypt() will skip its loop
and return 0, converting the error into a success:

arch/s390/crypto/aes_s390.c:__ctr_aes_crypt() {
	int ret = 0;
	while ((nbytes = walk->nbytes) >= AES_BLOCK_SIZE) {
		...
	}
	return ret;
}

Could this result in callers silently treating unencrypted data as a
successful cryptographic operation?

> +
> +	nbytes = walk.nbytes;
>  	/*
>  	 * final block may be < AES_BLOCK_SIZE, copy only nbytes
>  	 */

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

[PATCH 2/3] s390/crypto: Rework ctr_paes_do_crypt() to remove conditional locking

[Heiko Carstens]

Clang's compiler based static context analysis does not work with locks
that are conditionally taken like in ctr_paes_do_crypt():

arch/s390/crypto/paes_s390.c:980:13: warning: mutex 'ctrblk_lock' is not held on every path through here
      [-Wthread-safety-analysis]
  980 |                 ctrptr = (n > AES_BLOCK_SIZE) ? ctrblk : walk->iv;
      |                           ^

Given that code which takes locks conditionally can be considered
suboptimal rework ctr_paes_do_crypt() to get rid of this.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/crypto/paes_s390.c | 104 ++++++++++++++++++++---------------
 1 file changed, 60 insertions(+), 44 deletions(-)

diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
index 8cfe6166c193..d4a9108d39f5 100644
--- a/arch/s390/crypto/paes_s390.c
+++ b/arch/s390/crypto/paes_s390.c
@@ -932,15 +932,63 @@ static inline unsigned int __ctrblk_init(u8 *ctrptr, u8 *iv, unsigned int nbytes
 	return n;
 }
 
+static int __ctr_paes_do_crypt(struct s390_paes_ctx *ctx,
+			       struct s390_pctr_req_ctx *req_ctx,
+			       bool tested, bool maysleep, bool locked)
+{
+	struct skcipher_walk *walk = &req_ctx->walk;
+	struct ctr_param *param = &req_ctx->param;
+	unsigned int nbytes, n, k;
+	u8 *ctrptr;
+	int rc = 0;
+
+	/*
+	 * Note that in case of partial processing or failure the walk
+	 * is NOT unmapped here. So a follow up task may reuse the walk
+	 * or in case of unrecoverable failure needs to unmap it.
+	 */
+	while ((nbytes = walk->nbytes) >= AES_BLOCK_SIZE) {
+		n = AES_BLOCK_SIZE;
+		if (nbytes >= 2 * AES_BLOCK_SIZE && locked)
+			n = __ctrblk_init(ctrblk, walk->iv, nbytes);
+		ctrptr = (n > AES_BLOCK_SIZE) ? ctrblk : walk->iv;
+		k = cpacf_kmctr(ctx->fc, param, walk->dst.virt.addr,
+				walk->src.virt.addr, n, ctrptr);
+		if (k) {
+			if (ctrptr == ctrblk) {
+				memcpy(walk->iv, ctrptr + k - AES_BLOCK_SIZE,
+				       AES_BLOCK_SIZE);
+			}
+			crypto_inc(walk->iv, AES_BLOCK_SIZE);
+			rc = skcipher_walk_done(walk, nbytes - k);
+		}
+		if (k < n) {
+			if (!maysleep) {
+				rc = -EKEYEXPIRED;
+				goto out;
+			}
+			rc = paes_convert_key(ctx, tested);
+			if (rc)
+				goto out;
+			spin_lock_bh(&ctx->pk_lock);
+			memcpy(param->key, ctx->pk.protkey, sizeof(param->key));
+			spin_unlock_bh(&ctx->pk_lock);
+		}
+	}
+
+out:
+	return rc;
+}
+
 static int ctr_paes_do_crypt(struct s390_paes_ctx *ctx,
 			     struct s390_pctr_req_ctx *req_ctx,
 			     bool tested, bool maysleep)
 {
-	struct ctr_param *param = &req_ctx->param;
 	struct skcipher_walk *walk = &req_ctx->walk;
-	u8 buf[AES_BLOCK_SIZE], *ctrptr;
-	unsigned int nbytes, n, k;
-	int pk_state, locked, rc = 0;
+	struct ctr_param *param = &req_ctx->param;
+	u8 buf[AES_BLOCK_SIZE];
+	int pk_state, rc = 0;
+	unsigned int nbytes;
 
 	if (!req_ctx->param_init_done) {
 		/* fetch and check protected key state */
@@ -966,49 +1014,17 @@ static int ctr_paes_do_crypt(struct s390_paes_ctx *ctx,
 	if (rc)
 		goto out;
 
-	locked = mutex_trylock(&ctrblk_lock);
-
-	/*
-	 * Note that in case of partial processing or failure the walk
-	 * is NOT unmapped here. So a follow up task may reuse the walk
-	 * or in case of unrecoverable failure needs to unmap it.
-	 */
-	while ((nbytes = walk->nbytes) >= AES_BLOCK_SIZE) {
-		n = AES_BLOCK_SIZE;
-		if (nbytes >= 2 * AES_BLOCK_SIZE && locked)
-			n = __ctrblk_init(ctrblk, walk->iv, nbytes);
-		ctrptr = (n > AES_BLOCK_SIZE) ? ctrblk : walk->iv;
-		k = cpacf_kmctr(ctx->fc, param, walk->dst.virt.addr,
-				walk->src.virt.addr, n, ctrptr);
-		if (k) {
-			if (ctrptr == ctrblk)
-				memcpy(walk->iv, ctrptr + k - AES_BLOCK_SIZE,
-				       AES_BLOCK_SIZE);
-			crypto_inc(walk->iv, AES_BLOCK_SIZE);
-			rc = skcipher_walk_done(walk, nbytes - k);
-		}
-		if (k < n) {
-			if (!maysleep) {
-				if (locked)
-					mutex_unlock(&ctrblk_lock);
-				rc = -EKEYEXPIRED;
-				goto out;
-			}
-			rc = paes_convert_key(ctx, tested);
-			if (rc) {
-				if (locked)
-					mutex_unlock(&ctrblk_lock);
-				goto out;
-			}
-			spin_lock_bh(&ctx->pk_lock);
-			memcpy(param->key, ctx->pk.protkey, sizeof(param->key));
-			spin_unlock_bh(&ctx->pk_lock);
-		}
-	}
-	if (locked)
+	if (mutex_trylock(&ctrblk_lock)) {
+		rc = __ctr_paes_do_crypt(ctx, req_ctx, tested, maysleep, true);
 		mutex_unlock(&ctrblk_lock);
+	} else {
+		rc = __ctr_paes_do_crypt(ctx, req_ctx, tested, maysleep, false);
+	}
+	if (rc)
+		goto out;
 
 	/* final block may be < AES_BLOCK_SIZE, copy only nbytes */
+	nbytes = walk->nbytes;
 	if (nbytes) {
 		memset(buf, 0, AES_BLOCK_SIZE);
 		memcpy(buf, walk->src.virt.addr, nbytes);
-- 
2.53.0

Re: [PATCH 2/3] s390/crypto: Rework ctr_paes_do_crypt() to remove conditional locking

[sashiko-bot]

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

Pre-existing issues:
- [High] Silent data corruption when skcipher_walk_done() mapping errors are overwritten by paes_convert_key() false success.
--

--- Patch [2]: [PATCH 2/3] s390/crypto: Rework ctr_paes_do_crypt() to remove conditional locking ---
commit fd135aacfa5dd1086ab5711e4513f6e0edede1dc
Author: Heiko Carstens <hca@linux.ibm.com>

s390/crypto: Rework ctr_paes_do_crypt() to remove conditional locking

This commit extracts the core loop of ctr_paes_do_crypt() into a new
helper function, __ctr_paes_do_crypt(), to avoid conditionally taken locks
that trip up Clang's static analysis.

> diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
> index 8cfe6166c193c..d4a9108d39f5f 100644
> --- a/arch/s390/crypto/paes_s390.c
> +++ b/arch/s390/crypto/paes_s390.c

[ ... ]

> @@ -981,34 +955,76 @@ static int ctr_paes_do_crypt(struct s390_paes_ctx *ctx,
>  		k = cpacf_kmctr(ctx->fc, param, walk->dst.virt.addr,
>  				walk->src.virt.addr, n, ctrptr);
>  		if (k) {
> -			if (ctrptr == ctrblk)
> +			if (ctrptr == ctrblk) {
>  				memcpy(walk->iv, ctrptr + k - AES_BLOCK_SIZE,
>  				       AES_BLOCK_SIZE);
> +			}
>  			crypto_inc(walk->iv, AES_BLOCK_SIZE);
>  			rc = skcipher_walk_done(walk, nbytes - k);
>  		}
>  		if (k < n) {
>  			if (!maysleep) {
> -				if (locked)
> -					mutex_unlock(&ctrblk_lock);
>  				rc = -EKEYEXPIRED;
>  				goto out;
>  			}
>  			rc = paes_convert_key(ctx, tested);

[Severity: High]
This is a pre-existing issue, but does this sequence overwrite an error
from skcipher_walk_done()?

If the hardware stops processing early (k < n) and skcipher_walk_done() fails
to map the next scatterlist chunk, it can return a negative error code
like -ENOMEM into rc.

However, execution then immediately enters the if (k < n) block where rc is
unconditionally overwritten. If !maysleep, rc becomes -EKEYEXPIRED. If
paes_convert_key() is called and succeeds, rc becomes 0.

Could this result in silent data corruption where an incomplete encryption is
treated as a success (rc = 0) by the caller?

> -			if (rc) {
> -				if (locked)
> -					mutex_unlock(&ctrblk_lock);
> +			if (rc)
>  				goto out;
> -			}
>  			spin_lock_bh(&ctx->pk_lock);

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

Re: [PATCH 2/3] s390/crypto: Rework ctr_paes_do_crypt() to remove conditional locking

[Heiko Carstens]

Harald, Holger,

> Pre-existing issues:
> - [High] Silent data corruption when skcipher_walk_done() mapping errors are overwritten by paes_convert_key() false success.
> --
> 
> --- Patch [2]: [PATCH 2/3] s390/crypto: Rework ctr_paes_do_crypt() to remove conditional locking ---
> commit fd135aacfa5dd1086ab5711e4513f6e0edede1dc
> Author: Heiko Carstens <hca@linux.ibm.com>
> 
> s390/crypto: Rework ctr_paes_do_crypt() to remove conditional locking
> 
> This commit extracts the core loop of ctr_paes_do_crypt() into a new
> helper function, __ctr_paes_do_crypt(), to avoid conditionally taken locks
> that trip up Clang's static analysis.
> 
> > diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
> > index 8cfe6166c193c..d4a9108d39f5f 100644
> > --- a/arch/s390/crypto/paes_s390.c
> > +++ b/arch/s390/crypto/paes_s390.c
> 
> [ ... ]
> 
> > @@ -981,34 +955,76 @@ static int ctr_paes_do_crypt(struct s390_paes_ctx *ctx,
> >  		k = cpacf_kmctr(ctx->fc, param, walk->dst.virt.addr,
> >  				walk->src.virt.addr, n, ctrptr);
> >  		if (k) {
> > -			if (ctrptr == ctrblk)
> > +			if (ctrptr == ctrblk) {
> >  				memcpy(walk->iv, ctrptr + k - AES_BLOCK_SIZE,
> >  				       AES_BLOCK_SIZE);
> > +			}
> >  			crypto_inc(walk->iv, AES_BLOCK_SIZE);
> >  			rc = skcipher_walk_done(walk, nbytes - k);
> >  		}
> >  		if (k < n) {
> >  			if (!maysleep) {
> > -				if (locked)
> > -					mutex_unlock(&ctrblk_lock);
> >  				rc = -EKEYEXPIRED;
> >  				goto out;
> >  			}
> >  			rc = paes_convert_key(ctx, tested);
> 
> [Severity: High]
> This is a pre-existing issue, but does this sequence overwrite an error
> from skcipher_walk_done()?

What should happen with this? Can this be ignored? Or should this be
fixed like

  			rc = skcipher_walk_done(walk, nbytes - k);
			if (rc) {
				if (locked)
					mutex_unlock(&ctrblk_lock);
 				goto out;
			}

before the conversion?

Even though I said I don't want to fix pre-existing issues, I broke a
similar code path with the first patch, so...

Re: [PATCH 2/3] s390/crypto: Rework ctr_paes_do_crypt() to remove conditional locking

[Harald Freudenberger]

On 2026-08-04 14:47, Heiko Carstens wrote:
> Harald, Holger,
> 
>> Pre-existing issues:
>> - [High] Silent data corruption when skcipher_walk_done() mapping 
>> errors are overwritten by paes_convert_key() false success.
>> --
>> 
>> --- Patch [2]: [PATCH 2/3] s390/crypto: Rework ctr_paes_do_crypt() to 
>> remove conditional locking ---
>> commit fd135aacfa5dd1086ab5711e4513f6e0edede1dc
>> Author: Heiko Carstens <hca@linux.ibm.com>
>> 
>> s390/crypto: Rework ctr_paes_do_crypt() to remove conditional locking
>> 
>> This commit extracts the core loop of ctr_paes_do_crypt() into a new
>> helper function, __ctr_paes_do_crypt(), to avoid conditionally taken 
>> locks
>> that trip up Clang's static analysis.
>> 
>> > diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
>> > index 8cfe6166c193c..d4a9108d39f5f 100644
>> > --- a/arch/s390/crypto/paes_s390.c
>> > +++ b/arch/s390/crypto/paes_s390.c
>> 
>> [ ... ]
>> 
>> > @@ -981,34 +955,76 @@ static int ctr_paes_do_crypt(struct s390_paes_ctx *ctx,
>> >  		k = cpacf_kmctr(ctx->fc, param, walk->dst.virt.addr,
>> >  				walk->src.virt.addr, n, ctrptr);
>> >  		if (k) {
>> > -			if (ctrptr == ctrblk)
>> > +			if (ctrptr == ctrblk) {
>> >  				memcpy(walk->iv, ctrptr + k - AES_BLOCK_SIZE,
>> >  				       AES_BLOCK_SIZE);
>> > +			}
>> >  			crypto_inc(walk->iv, AES_BLOCK_SIZE);
>> >  			rc = skcipher_walk_done(walk, nbytes - k);
>> >  		}
>> >  		if (k < n) {
>> >  			if (!maysleep) {
>> > -				if (locked)
>> > -					mutex_unlock(&ctrblk_lock);
>> >  				rc = -EKEYEXPIRED;
>> >  				goto out;
>> >  			}
>> >  			rc = paes_convert_key(ctx, tested);
>> 
>> [Severity: High]
>> This is a pre-existing issue, but does this sequence overwrite an 
>> error
>> from skcipher_walk_done()?
> 
> What should happen with this? Can this be ignored? Or should this be
> fixed like
> 
>   			rc = skcipher_walk_done(walk, nbytes - k);
> 			if (rc) {
> 				if (locked)
> 					mutex_unlock(&ctrblk_lock);
>  				goto out;
> 			}
> 
> before the conversion?
> 
> Even though I said I don't want to fix pre-existing issues, I broke a
> similar code path with the first patch, so...

I'll take care of this and let you know within 1 day. I opened up 
bugzilla
https://bugzilla.linux.ibm.com/show_bug.cgi?id=220960 for tracking.

[PATCH 3/3] s390/crypto: Enable CONTEXT_ANALYSIS

[Heiko Carstens]

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

Signed-off-by: Heiko Carstens <hca@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.53.0

Re: [PATCH 3/3] s390/crypto: Enable CONTEXT_ANALYSIS

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing error check for `skcipher_walk_virt()` in `ctr_aes_crypt()` causes errors to be silently swallowed.
--

--- Patch [3]: [PATCH 3/3] s390/crypto: Enable CONTEXT_ANALYSIS ---
commit 2232f8277d7ab7cfce4ac97a8923bcf8c4ed60e3
Author: Heiko Carstens <hca@linux.ibm.com>

s390/crypto: Enable CONTEXT_ANALYSIS

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

> diff --git a/arch/s390/crypto/Makefile b/arch/s390/crypto/Makefile
> index 48aeb0c0ffbd6..1d64208139357 100644
> --- a/arch/s390/crypto/Makefile
> +++ b/arch/s390/crypto/Makefile
> @@ -3,6 +3,8 @@
>  # Cryptographic API
>  #
>  
> +CONTEXT_ANALYSIS := y
> +

[Severity: High]
Does this code silently swallow errors from skcipher_walk_virt()?

In arch/s390/crypto/aes_s390.c:ctr_aes_crypt():

	ret = skcipher_walk_virt(&walk, req, false);

	if (mutex_trylock(&ctrblk_lock)) {
		ret = __ctr_aes_crypt(sctx, &walk, true);
		mutex_unlock(&ctrblk_lock);
	} else {
		ret = __ctr_aes_crypt(sctx, &walk, false);
	}

If memory allocation or scatterlist setup fails during skcipher_walk_virt(),
it returns an error code (such as -ENOMEM or -EINVAL). Because ret is
immediately overwritten by __ctr_aes_crypt() without being checked, the
caller may incorrectly receive 0 (success).

This causes a silent failure where unencrypted or undecrypted data may be
consumed by the caller.

Could we add an error check before calling __ctr_aes_crypt()?

>  obj-$(CONFIG_CRYPTO_AES_S390) += aes_s390.o

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

Re: [PATCH 0/3] s390/crypto: Enable CONTEXT_ANALYSIS

[Herbert Xu]

On Tue, Aug 04, 2026 at 01:37:33PM +0200, Heiko Carstens wrote:
> Enable CONTEXT_ANALYSYS for s390's crypto code.
> 
> Static code checking for acquiring and releasing locks used to be done
> with sparse. That was removed with [1] and replaced with a clang based
> approach [2]. The new approach requires that each subsystem needs to be
> explicitly enabled for checking.
> 
> Do that for s390's crypto code. In order to avoid false positives the
> code has to be slightly reworked, since conditionally acquiring and
> releasing locks does not work with the checker (besides that this is
> sub optimal coding style).
> 
> [1] 5b63d0ae94cc ("compiler-context-analysis: Remove Sparse support")
> [2] 3269701cb256 ("compiler-context-analysis: Add infrastructure for Context Analysis with Clang")
> 
> Note:
> 
> - "Pre-existing issues" reported by AI will not be addressed in the
>   context of this series
> 
> Heiko Carstens (3):
>   s390/crypto: Rework ctr_aes_crypt() to remove conditional locking
>   s390/crypto: Rework ctr_paes_do_crypt() to remove conditional locking
>   s390/crypto: Enable CONTEXT_ANALYSIS
> 
>  arch/s390/crypto/Makefile    |   2 +
>  arch/s390/crypto/aes_s390.c  |  54 +++++++++++-------
>  arch/s390/crypto/paes_s390.c | 104 ++++++++++++++++++++---------------
>  3 files changed, 97 insertions(+), 63 deletions(-)

I presume this is going through the s390 tree?

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

Re: [PATCH 0/3] s390/crypto: Enable CONTEXT_ANALYSIS

[Harald Freudenberger]

On 2026-08-15 02:59, Herbert Xu wrote:
> On Tue, Aug 04, 2026 at 01:37:33PM +0200, Heiko Carstens wrote:
>> Enable CONTEXT_ANALYSYS for s390's crypto code.
>> 
>> Static code checking for acquiring and releasing locks used to be done
>> with sparse. That was removed with [1] and replaced with a clang based
>> approach [2]. The new approach requires that each subsystem needs to 
>> be
>> explicitly enabled for checking.
>> 
>> Do that for s390's crypto code. In order to avoid false positives the
>> code has to be slightly reworked, since conditionally acquiring and
>> releasing locks does not work with the checker (besides that this is
>> sub optimal coding style).
>> 
>> [1] 5b63d0ae94cc ("compiler-context-analysis: Remove Sparse support")
>> [2] 3269701cb256 ("compiler-context-analysis: Add infrastructure for 
>> Context Analysis with Clang")
>> 
>> Note:
>> 
>> - "Pre-existing issues" reported by AI will not be addressed in the
>>   context of this series
>> 
>> Heiko Carstens (3):
>>   s390/crypto: Rework ctr_aes_crypt() to remove conditional locking
>>   s390/crypto: Rework ctr_paes_do_crypt() to remove conditional 
>> locking
>>   s390/crypto: Enable CONTEXT_ANALYSIS
>> 
>>  arch/s390/crypto/Makefile    |   2 +
>>  arch/s390/crypto/aes_s390.c  |  54 +++++++++++-------
>>  arch/s390/crypto/paes_s390.c | 104 
>> ++++++++++++++++++++---------------
>>  3 files changed, 97 insertions(+), 63 deletions(-)
> 
> I presume this is going through the s390 tree?
> 
> Cheers,

We can do this via s390 subsystem. I anyway wanted to suggest this.
Thanks