arch/s390/crypto/sha1_s390.c | 1 + arch/s390/crypto/sha512_s390.c | 2 ++ 2 files changed, 3 insertions(+)
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
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
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/
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
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>
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>
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/
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
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
© 2016 - 2025 Red Hat, Inc.