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

Eric Biggers posted 1 patch 3 months, 1 week ago
There is a newer version of this series
arch/s390/crypto/sha1_s390.c   | 1 +
arch/s390/crypto/sha512_s390.c | 2 ++
2 files changed, 3 insertions(+)
[PATCH] crypto: s390/sha - Fix uninitialized variable in SHA-1 and SHA-2
Posted by Eric Biggers 3 months, 1 week ago
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; 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")
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
-- 
2.50.0
Re: [PATCH] crypto: s390/sha - Fix uninitialized variable in SHA-1 and SHA-2
Posted by Eric Biggers 3 months ago
On Fri, Jun 27, 2025 at 11:56:49AM -0700, 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; 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")
> 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(+)

I just realized this patch is incomplete: it updated s390_sha1_init(),
sha384_init(), and sha512_init(), but not s390_sha1_import() and sha512_import()
which need the same fix...  I'll send a v2.

- Eric
Re: [PATCH] crypto: s390/sha - Fix uninitialized variable in SHA-1 and SHA-2
Posted by Ingo Franzki 3 months ago
On 03.07.2025 19:20, Eric Biggers wrote:
> On Fri, Jun 27, 2025 at 11:56:49AM -0700, 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; 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")
>> 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(+)
> 
> I just realized this patch is incomplete: it updated s390_sha1_init(),
> sha384_init(), and sha512_init(), but not s390_sha1_import() and sha512_import()
> which need the same fix...  I'll send a v2.

Good finding. Yes the import functions also need the fix.
Your updates in "[PATCH v2] crypto: s390/sha - Fix uninitialized variable in SHA-1 and SHA-2" look good.

> 
> - Eric


-- 
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/
Re: [PATCH] crypto: s390/sha - Fix uninitialized variable in SHA-1 and SHA-2
Posted by Eric Biggers 3 months, 1 week ago
On Fri, Jun 27, 2025 at 11:56:49AM -0700, 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; 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")
> 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.

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=libcrypto-fixes

- Eric
Re: [PATCH] crypto: s390/sha - Fix uninitialized variable in SHA-1 and SHA-2
Posted by Eric Biggers 3 months, 1 week ago
On Mon, Jun 30, 2025 at 09:58:05AM -0700, Eric Biggers wrote:
> On Fri, Jun 27, 2025 at 11:56:49AM -0700, 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; 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")
> > 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.
> 
> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=libcrypto-fixes

Forgot to mention: I revised the first two paragraphs of the commit message to
fix a couple things and clarify that the accidental CPACF_KIMD_NIP was indeed
ignored (as per Ingo):

    crypto: s390/sha - Fix uninitialized variable in SHA-1 and SHA-2
    
    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() (now s390_sha_update_blocks()).  At the time,
    s390_sha_update() 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. the function code CPACF_KIMD_SHA_512 |
    CPACF_KIMD_NIP to be used instead of just CPACF_KIMD_SHA_512.  This
    apparently was harmless, as the SHA-1 and SHA-2 function codes ignore
    CPACF_KIMD_NIP; it is recognized only by the SHA-3 function codes
    (https://lore.kernel.org/r/73477fe9-a1dc-4e38-98a6-eba9921e8afa@linux.ibm.com/).
    Therefore, this bug was found only when first_message_part was later
    converted to a boolean and UBSAN detected its uninitialized use.
    Regardless, let's fix this by just initializing to false.
    
    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")
    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
    Acked-by: Heiko Carstens <hca@linux.ibm.com>
    Reviewed-by: Ingo Franzki <ifranzki@linux.ibm.com>
    Link: https://lore.kernel.org/r/20250627185649.35321-1-ebiggers@kernel.org
    Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Re: [PATCH] crypto: s390/sha - Fix uninitialized variable in SHA-1 and SHA-2
Posted by Heiko Carstens 3 months, 1 week ago
On Fri, Jun 27, 2025 at 11:56:49AM -0700, 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; 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")
> 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(+)

Routing this via libcrypto-fixes is fine.

Acked-by: Heiko Carstens <hca@linux.ibm.com>
Re: [PATCH] crypto: s390/sha - Fix uninitialized variable in SHA-1 and SHA-2
Posted by Ingo Franzki 3 months, 1 week ago
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/
Re: [PATCH] crypto: s390/sha - Fix uninitialized variable in SHA-1 and SHA-2
Posted by Eric Biggers 3 months, 1 week ago
On Mon, Jun 30, 2025 at 08:26:34AM +0200, Ingo Franzki wrote:
> 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.
> 

Ah, that explains it then.

> > 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. 

Yes, when backporting this patch to 6.15 and 6.12 there will be 2 things that
I'll change:

1. Extend it to cover SHA-224 and SHA-256 (which had been reworked in 6.16)
2. Change false to 0

- Eric
Re: [PATCH] crypto: s390/sha - Fix uninitialized variable in SHA-1 and SHA-2
Posted by Eric Biggers 3 months, 1 week ago
On Fri, Jun 27, 2025 at 11:56:49AM -0700, Eric Biggers wrote:
> This could cause e.g. CPACF_KIMD_SHA_512 | CPACF_KIMD_NIP to be used
> instead of just CPACF_KIMD_NIP.

That should say "instead of just CPACF_KIMD_SHA_512".

- Eric