Re: [PATCH] crypto: s390/sha - Fix uninitialized variable in SHA-1 and SHA-2

[Ingo Franzki <ifranzki@linux.ibm.com>]

On 27.06.2025 20:56, Eric Biggers wrote:
> Commit 88c02b3f79a6 ("s390/sha3: Support sha3 performance enhancements")
> added the field s390_sha_ctx::first_message_part and made it be used by
> s390_sha_update_blocks().  At the time, s390_sha_update_blocks() was
> used by all the s390 SHA-1, SHA-2, and SHA-3 algorithms.  However, only
> the initialization functions for SHA-3 were updated, leaving SHA-1 and
> SHA-2 using first_message_part uninitialized.
> 
> This could cause e.g. CPACF_KIMD_SHA_512 | CPACF_KIMD_NIP to be used
> instead of just CPACF_KIMD_NIP.  It's unclear why this didn't cause a
> problem earlier; 

The NIP flag is only recognized by the SHA3 function codes if the KIMD instruction, for the others (SHA1 and SHA2) it is ignored.

this bug was found only when UBSAN detected the
> uninitialized boolean.  Perhaps the CPU ignores CPACF_KIMD_NIP for SHA-1
> and SHA-2.  Regardless, let's fix this.  For now just initialize to
> false, i.e. don't try to "optimize" the SHA state initialization.
> 
> Note: in 6.16, we need to patch SHA-1, SHA-384, and SHA-512.  In 6.15
> and earlier, we'll also need to patch SHA-224 and SHA-256, as they
> hadn't yet been librarified (which incidentally fixed this bug).
> 
> Fixes: 88c02b3f79a6 ("s390/sha3: Support sha3 performance enhancements")

If this patch is applied on 88c02b3f79a6 then the first_message_part field should
probably set to 0 instead of false, since only since commit 
7b83638f962c30cb6271b5698dc52cdf9b638b48 "crypto: s390/sha1 - Use API partial block handling"
first_message_part is a bool, before it was an int. 

> Cc: stable@vger.kernel.org
> Reported-by: Ingo Franzki <ifranzki@linux.ibm.com>
> Closes: https://lore.kernel.org/r/12740696-595c-4604-873e-aefe8b405fbf@linux.ibm.com
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---
> 
> This is targeting 6.16.  I'd prefer to take this through
> libcrypto-fixes, since the librarification work is also touching this
> area.  But let me know if there's a preference for the crypto tree or
> the s390 tree instead.
> 
>  arch/s390/crypto/sha1_s390.c   | 1 +
>  arch/s390/crypto/sha512_s390.c | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/arch/s390/crypto/sha1_s390.c b/arch/s390/crypto/sha1_s390.c
> index d229cbd2ba229..73672e76a88f9 100644
> --- a/arch/s390/crypto/sha1_s390.c
> +++ b/arch/s390/crypto/sha1_s390.c
> @@ -36,10 +36,11 @@ static int s390_sha1_init(struct shash_desc *desc)
>  	sctx->state[2] = SHA1_H2;
>  	sctx->state[3] = SHA1_H3;
>  	sctx->state[4] = SHA1_H4;
>  	sctx->count = 0;
>  	sctx->func = CPACF_KIMD_SHA_1;
> +	sctx->first_message_part = false;
>  
>  	return 0;
>  }
>  
>  static int s390_sha1_export(struct shash_desc *desc, void *out)
> diff --git a/arch/s390/crypto/sha512_s390.c b/arch/s390/crypto/sha512_s390.c
> index 33711a29618c3..e9e112025ff22 100644
> --- a/arch/s390/crypto/sha512_s390.c
> +++ b/arch/s390/crypto/sha512_s390.c
> @@ -30,10 +30,11 @@ static int sha512_init(struct shash_desc *desc)
>  	ctx->sha512.state[6] = SHA512_H6;
>  	ctx->sha512.state[7] = SHA512_H7;
>  	ctx->count = 0;
>  	ctx->sha512.count_hi = 0;
>  	ctx->func = CPACF_KIMD_SHA_512;
> +	ctx->first_message_part = false;
>  
>  	return 0;
>  }
>  
>  static int sha512_export(struct shash_desc *desc, void *out)
> @@ -95,10 +96,11 @@ static int sha384_init(struct shash_desc *desc)
>  	ctx->sha512.state[6] = SHA384_H6;
>  	ctx->sha512.state[7] = SHA384_H7;
>  	ctx->count = 0;
>  	ctx->sha512.count_hi = 0;
>  	ctx->func = CPACF_KIMD_SHA_512;
> +	ctx->first_message_part = false;
>  
>  	return 0;
>  }
>  
>  static struct shash_alg sha384_alg = {
> 
> base-commit: e540341508ce2f6e27810106253d5de194b66750

Reviewed-by: Ingo Franzki <ifranzki@linux.ibm.com>


-- 
Ingo Franzki
eMail: ifranzki@linux.ibm.com  
Tel: ++49 (0)7031-16-4648
Linux on IBM Z Development, Schoenaicher Str. 220, 71032 Boeblingen, Germany

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM DATA Privacy Statement: https://www.ibm.com/privacy/us/en/