linux-kernel - Re: [PATCH v2 0/9] crypto: x86

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250402171930.GD1235@sol.localdomain>
Date: Wed, 2 Apr 2025 10:19:30 -0700
From: Eric Biggers <ebiggers@...nel.org>
To: Herbert Xu <herbert@...dor.apana.org.au>
Cc: Ard Biesheuvel <ardb@...nel.org>, linux-crypto@...r.kernel.org,
	linux-kernel@...r.kernel.org, x86@...nel.org,
	"Jason A. Donenfeld" <Jason@...c4.com>
Subject: Re: [PATCH v2 0/9] crypto: x86 - stop using the SIMD helper

On Wed, Apr 02, 2025 at 04:22:21PM +0800, Herbert Xu wrote:
> On Wed, Apr 02, 2025 at 09:34:30AM +0300, Ard Biesheuvel wrote:
> >
> > Ah, never mind - I see some calls on 32-bit ARM to
> > simd_skcipher_create_compat(), which have become redundant now that
> > SIMD is guaranteed to be available in softirq context.
> 
> Thanks!
> 
> We could also remove all the calls to crypto_simd_usable in the
> Crypto API hashing code, e.g., arch/arm64/crypto/sha1-ce-glue.c.
> 
> For the lib/crypto code I think we should make it a rule to
> not allow any hardirq usage just like the Crypto API.  Does
> anyone know of any uses of lib/crypto in a hardirq?

This seems premature.  crypto_shash is documented to be usable in any context.
See the "Context:" comments in include/crypto/hash.h.  Similarly, developers
expect lib/ functions to be available in any context unless otherwise
documented.

For skcipher and aead, there are more reasons why it makes sense to limit the
contexts:

- skcipher_walk_first() already explicitly errors out if in_hardirq(), which
  already prevents them from working in hardirq context in most cases
- Even if it was allowed, the skcipher and aead APIs are already difficult to
  use correctly in a hardirq
- Because of how the crypto API is designed, it's not straightforward to fall
  back to generic skcipher and aead code in no-SIMD contexts

I could see the limitation being brought into crypto_shash too, though the
crypto_shash documentation will need to be updated.  The crypto API also really
needs to be explicitly checking all its requirements.  (It's probably finally
time to add a kconfig option like CONFIG_DEBUG_CRYPTO to lib/Kconfig.debug, and
put the extra assertions under there.  Then they could be added without
impacting performance for normal users.)

IMO, doing it for lib/ too would be going too far though.  The lib/ functions
should be easy to use and not have random requirements on the calling context.
And since they're just functions, it's easy for them to fall back to the generic
functions when needed.  Also note that for very short inputs it can actually be
faster to use no-SIMD code, as that avoids the overhead of a kernel-mode SIMD
section.  So the fallback sometimes exists anyway for that.

- Eric