linux-kernel - Re: [v2 PATCH 00/13] Architecture-optimized SHA-256 library API

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: <20250427131138.GC1161@quark>
Date: Sun, 27 Apr 2025 06:11:38 -0700
From: Eric Biggers <ebiggers@...nel.org>
To: Herbert Xu <herbert@...dor.apana.org.au>
Cc: Linux Crypto Mailing List <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, Ard Biesheuvel <ardb@...nel.org>,
	"Jason A . Donenfeld " <Jason@...c4.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [v2 PATCH 00/13] Architecture-optimized SHA-256 library API

On Sun, Apr 27, 2025 at 05:56:43AM -0700, Eric Biggers wrote:
> On Sun, Apr 27, 2025 at 08:41:38PM +0800, Herbert Xu wrote:
> > On Sun, Apr 27, 2025 at 05:35:14AM -0700, Eric Biggers wrote:
> > >
> > > Well, barely a day and you've already ruined my patch series.  Now instead of a
> > > clean design where the crypto_shash API is built on top of the normal library
> > > API (sha256_update() etc.), there's now a special low-level API
> > > "sha256_choose_blocks()" just for shash that it's built on top of instead, for
> > > no good reason.  You're also still pushing your broken BLOCK_HASH_UPDATE_BLOCKS
> > > macro that doesn't work with size_t, and putting my name on your broken code
> > > that uses it.
> > 
> > Your design is unacceptable because you're forcing the partial block
> > handling on shash where it's not needed,
> 
> Excuse me?  It's the other way around.  In my version the partial block handling
> is only in the library, not shash.  In your version you've forced it into the
> shash layer, even though the library does it already.  I understand that you've
> added support for partial block handling to crypto/shash.c and you want to feel
> like your work is useful, but in this case it's not, since the libray has to
> handle arbitrary-length inputs anyway.
> 
> > just as you're forcing the hardirq support on everything.
> 
> If you want crypto_shash to warn on hardirq usage you should just put a
> WARN_ON(in_hardirq()) in crypto_shash_*(), which will actually achieve that.
> Not add a shash-specific non-hardirq-safe low-level API to the library that can
> silently corrupt random tasks' SIMD registers on production systems.

By the way, as I mentioned in my cover letter:

    For now the SHA-256 library is well-covered by the crypto_shash
    self-tests, but I plan to add a test for the library directly later.

But due to your gratuitous changes where crypto_shash is no longer built on top
of the normal SHA-256 library API, that's no longer the case.

So while I do still plan to add a SHA-256 library test anyway, I don't see the
reason for not also making crypto_shash just do the right thing.

- Eric