linux-kernel - Re: [PATCH 11/13] crypto: x86/sha256 - implement library instead of shash

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: <aA138IKjqyZeQLgB@gondor.apana.org.au>
Date: Sun, 27 Apr 2025 08:18:56 +0800
From: Herbert Xu <herbert@...dor.apana.org.au>
To: Eric Biggers <ebiggers@...nel.org>
Cc: linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arch@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-mips@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
	linux-riscv@...ts.infradead.org, sparclinux@...r.kernel.org,
	linux-s390@...r.kernel.org, x86@...nel.org, ardb@...nel.org,
	Jason@...c4.com, torvalds@...ux-foundation.org
Subject: Re: [PATCH 11/13] crypto: x86/sha256 - implement library instead of
 shash

On Sat, Apr 26, 2025 at 11:03:26AM -0700, Eric Biggers wrote:
>
> The SHA-256 library functions currently work in any context, and this patch
> series preserves that behavior.  Changing that would be a separate change.

I've already removed the SIMD fallback path and your patch is
adding it back.

> But also as I've explained before, for the library API the performance benefit
> of removing the crypto_simd_usable() doesn't seem to be worth the footgun that
> would be introduced.  Your position is, effectively, that if someone calls one
> of the sha256*() functions from a hardirq, we should sometimes corrupt a random
> task's FPU registers.  That's a really bad bug that is very difficult to
> root-cause.  My position is that we should make it just work as expected.

kernel_fpu_begin already does a WARN_ON when called in hardirq
context and it can't safely use the FPU, there is no silent
corruption.

In fact if anything your patch is making the problem worse by
making a hardirq stochastically slow with no visible warnings
at all.

> Yes, no one *should* be doing SHA-256 in a hardirq.  But I don't think that
> means we should corrupt a random task's FPU registers if someone doesn't follow
> best practices, when we can easily make the API just work as expected.

If you really want to support this, do it in the FPU layer, not here.

Thanks,
-- 
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt