[GIT PULL] Crypto Fixes for 6.17

[GIT PULL] Crypto Fixes for 6.17

[Herbert Xu]

Hi Linus:

The following changes since commit bf24d64268544379d9a9b5b8efc2bb03967703b3:

  crypto: keembay - Use min() to simplify ocs_create_linked_list_from_sg() (2025-07-27 22:41:45 +1000)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git tags/v6.17-p2

for you to fetch changes up to 9d9b193ed73a65ec47cf1fd39925b09da8216461:

  crypto: hash - Increase HASH_MAX_DESCSIZE for hmac(sha3-224-s390) (2025-08-01 19:40:54 +0800)

----------------------------------------------------------------
This push fixes a regression that breaks hmac(sha3-224-s390).
----------------------------------------------------------------

Herbert Xu (1):
      crypto: hash - Increase HASH_MAX_DESCSIZE for hmac(sha3-224-s390)

 include/crypto/hash.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Thanks,
-- 
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: [GIT PULL] Crypto Fixes for 6.17

[Linus Torvalds]

On Fri, 8 Aug 2025 at 08:42, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> This push fixes a regression that breaks hmac(sha3-224-s390).

_Please_ describe the completely random strange constants, and why they changed.

What is "361", and why did 360 use to work but no longer does?

I've pulled this, because I'm sure it fixes a bug, but neither the
pull message nor the commit have acceptable explanations.

And honestly, the code should be fixed too. Having a random constant
like that with no explanation for the completely random value is not
ok.

             Linus

Re: [GIT PULL] Crypto Fixes for 6.17

[pr-tracker-bot]

The pull request you sent on Fri, 8 Aug 2025 13:41:51 +0800:

> git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git tags/v6.17-p2

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/01b6ba6b097a0ceeef1975ae37c1660fed1b560c

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

Re: [GIT PULL] Crypto Fixes for 6.17

[Vegard Nossum]

On 09/08/2025 06:32, Linus Torvalds wrote:
> On Fri, 8 Aug 2025 at 08:42, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>>
>> This push fixes a regression that breaks hmac(sha3-224-s390).
> 
> _Please_ describe the completely random strange constants, and why they changed.
> 
> What is "361", and why did 360 use to work but no longer does?
> 
> I've pulled this, because I'm sure it fixes a bug, but neither the
> pull message nor the commit have acceptable explanations.
> 
> And honestly, the code should be fixed too. Having a random constant
> like that with no explanation for the completely random value is not
> ok.

The actual explanation is given in the email here:

On Wed, Jul 30, 2025 at 09:11:49AM -0700, Eric Biggers wrote:
> 
> I haven't touched SHA-3 yet.  This is a bug from the following
> commit:
> 
> commit 6f90ba7065515d69b24729cf85c45b2add99e638 Author: Herbert Xu
> <herbert@gondor.apana.org.au> Date:   Fri Apr 18 11:00:13 2025 +0800
> 
> crypto: s390/sha3 - Use API partial block handling
> 
> Use the Crypto API partial block handling.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> That increased the descsize of hmac(sha3-224-s390) from 368 to 369, 
> which made it exceed HASH_MAX_DESCSIZE, causing it to fail to
> register.
(https://lore.kernel.org/all/20250730161149.GA1162@sol/)

This is an anti-pattern of the crypto code that AFAICT ultimately stems
from the removal of VLAs:

commit b68a7ec1e9a3efac53ae26a1658a553825a2375c
Date:   Tue Aug 7 14:18:38 2018 -0700

     crypto: hash - Remove VLA usage

which replaced e.g. crypto_shash_descsize(ctx) by HASH_MAX_DESCSIZE, a
hard coded limit that's supposed to capture the biggest struct you can
possibly put on the stack (SHASH_DESC_ON_STACK() etc.) -- since the
crypto API is stringly typed you cannot know the exact size of the
thing you are requesting ahead of time (the sizes could vary depending
on which implementation the crypto API decides to use).

I call it an anti-pattern because it's not the first time this has had
bugs either:

commit e1354400b25da645c4764ed6844d12f1582c3b66
Date:   Tue May 14 16:13:15 2019 -0700

     crypto: hash - fix incorrect HASH_MAX_DESCSIZE

As a minimal future-proofing fix, maybe we could add something like

BUILD_BUG_ON(sizeof(struct md5_state) <= HASH_MAX_DESCSIZE);

to every hashing algorithm, and/or a dynamic check in the crypto API
(completely untested):

--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -361,6 +361,8 @@ int crypto_register_shash(struct shash_alg *alg)
         struct crypto_alg *base = &alg->base;
         int err;

+       WARN_ON(alg->descsize > HASH_MAX_DESCSIZE);
+
         err = shash_prepare_alg(alg);
         if (err)
                 return err;

...or maybe those on-stack users should just do the kmalloc and be done
with it.


Vegard

Re: [GIT PULL] Crypto Fixes for 6.17

[Linus Torvalds]

On Sat, 9 Aug 2025 at 21:22, Vegard Nossum <vegard.nossum@oracle.com> wrote:
>
> The actual explanation is given in the email here:

Yeah, that should have been in the commit message somewhere.

And honestly, it should have been in the code too. Having very random
constants in header files with no explanation for them is not great.

> This is an anti-pattern of the crypto code that AFAICT ultimately stems
> from the removal of VLAs:

I'd say that it stems from using random sizes with no logic and the
VLAs were just the *previous* problem case of the same issue.

> As a minimal future-proofing fix, maybe we could add something like
>
> BUILD_BUG_ON(sizeof(struct md5_state) <= HASH_MAX_DESCSIZE);
>
> to every hashing algorithm, and/or a dynamic check in the crypto API
> (completely untested):

The dynamic check may be the right thing to do regardless, but when
fixing outright bugs, at least document what went wrong and why. Not
just "360 was too small for X, so it is now 361".

                Linus

[GIT PULL] Crypto Fixes for 6.17

[Herbert Xu]

Hi Linus:

The following changes since commit 8f5ae30d69d7543eee0d70083daf4de8fe15d585:

  Linux 6.17-rc1 (2025-08-10 19:41:16 +0300)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git tags/v6.17-p3

for you to fetch changes up to 1b34cbbf4f011a121ef7b2d7d6e6920a036d5285:

  crypto: af_alg - Disallow concurrent writes in af_alg_sendmsg (2025-09-18 17:24:59 +0800)

----------------------------------------------------------------
This push fixes a NULL pointer dereference in ccp and a couple of
bugs in the af_alg interface.
----------------------------------------------------------------

Borislav Petkov (AMD) (1):
      crypto: ccp - Always pass in an error pointer to __sev_platform_shutdown_locked()

Herbert Xu (2):
      crypto: af_alg - Set merge to zero early in af_alg_sendmsg
      crypto: af_alg - Disallow concurrent writes in af_alg_sendmsg

 crypto/af_alg.c              | 10 +++++++++-
 drivers/crypto/ccp/sev-dev.c |  2 +-
 include/crypto/if_alg.h      | 10 ++++++----
 3 files changed, 16 insertions(+), 6 deletions(-)

Thanks,
-- 
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: [GIT PULL] Crypto Fixes for 6.17

[pr-tracker-bot]

The pull request you sent on Fri, 19 Sep 2025 12:10:03 +0800:

> git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git tags/v6.17-p3

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/dcf7d9e0aee523e588aa3d5ce7394043cd2dea9e

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

Re: [GIT PULL] Crypto Fixes for 6.17

[Eric Biggers]

On Fri, Sep 19, 2025 at 12:10:03PM +0800, Herbert Xu wrote:
> Herbert Xu (2):
>       crypto: af_alg - Set merge to zero early in af_alg_sendmsg
>       crypto: af_alg - Disallow concurrent writes in af_alg_sendmsg

These seem to have been pushed out without any public review.  Note that
the second patch is buggy, since it changed fields from 'bool' to 1-bit
bitfields without updating code to stop assigning values greater than 1.

- Eric

Re: [GIT PULL] Crypto Fixes for 6.17

[Gu Bowen]

Hi, 
On Fri, Sep 19, 2025 at 12:10:03PM +0800, Herbert Xu wrote:
> Herbert Xu (2):
>       crypto: af_alg - Set merge to zero early in af_alg_sendmsg

I have a question about this patch: is the "fixes" tag not quite
appropriate? In older kernel versions, such as v5.10, MSG_SPLICE_PAGES
is not yet supported, so there is no need for ctx->merge = 0. Perhaps
d3dccb0a487d ("crypto: af_alg - Fix merging of written data into spliced
pages") would be more suitable.

Best Regards,
Guber