Message-ID: <20250628011834.GA1246405@google.com>
Date: Sat, 28 Jun 2025 01:18:34 +0000
From: Eric Biggers <ebiggers@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
	Ard Biesheuvel <ardb@...nel.org>,
	"Jason A. Donenfeld" <Jason@...c4.com>,
	Arnd Bergmann <arnd@...db.de>
Subject: Re: [GIT PULL] Crypto library fix for v6.16-rc4

On Fri, Jun 27, 2025 at 05:54:05PM -0700, Linus Torvalds wrote:
> On Fri, 27 Jun 2025 at 11:15, Eric Biggers <ebiggers@...nel.org> wrote:
> >
> > Fix a regression where the purgatory code sometimes fails to build.
> 
> Hmm. This is obviously a fine and simple fix, but at the same time it
> smells to me that the underlying problem here is  that the purgatory
> code is just too damn fragile, and is being very incestuous with the
> sha2 code.
> 
> The purgatory code tends to be really special in so many other ways
> too (if you care, just look at how it plays games with compiler flags
> because it also doesn't want tracing code etc).
> 
> And when it comes to the crypto code, it plays games with just
> re-building the sha256.c file inside the purgatory directory, and is
> just generallyt being pretty hacky.
> 
> Anyway, I've pulled this because as long as it fixes the issue and you
> are ok with dealing with this crazy code I think it's all good.
> 
> But I also get the feeling that this should be very much seen as a
> purgatory code problem, not a crypto library problem.
> 
> We seem to have the same hacks for risc-v, s390 and x86, and I wonder
> if the safe thing to do long-term from a maintenance sanity standpoint
> would be to just make the purgatory code hackery use the generic
> sha256 implementation.
> 
> I say that purely as a "maybe it's not a good idea to mix the crazy
> purgatory code with the special arch-specific optimized code that may
> need special infrastructure".
> 
> The fact that the x86 sha256 routines do that whole irq_fpu_usable()
> thing etc is a symptom of that kind of "the architecture code is
> special".
> 
> But as long as you are fine with maintaining that arch-optimized code
> knowing that it gets (mis-)used by the strange purgatory code, I
> certainly don't mind the status quo with that one-liner fix.
> 
> So I guess this email is just me saying "if this keeps triggering
> problems, just make the purgatory code use the slow generic routines".
> 
> Because it's not necessarily worth the pain to support arch-optimized
> versions for that case.
> 
> If there is pain, that is.

Purgatory actually gets the generic SHA-256 code already.  The way it works is
that for purgatory lib/crypto/sha256.c is built with __DISABLE_EXPORTS defined,
and that file detects that and disables the arch-optimized code.  The
arch-optimized assembly code is not built into purgatory.

This isn't particularly hard to continue supporting, versus the alternative of
duplicating the generic SHA-256 code into a special file that's just for
purgatory.  5b90a779bc547 just made it unnecessarily fragile by relying on
compiler inlining to avoid a call to the arch-optimized code (which isn't built
into purgatory) from being generated.

My series
https://lore.kernel.org/linux-crypto/20250625070819.1496119-1-ebiggers@kernel.org/
makes it simpler and less fragile.  The #include of sha256-generic.c from
sha256.c goes away, and the selection of sha256_blocks() becomes just:

    #if defined(CONFIG_CRYPTO_LIB_SHA256_ARCH) && !defined(__DISABLE_EXPORTS)
    #include "sha256.h" /* $(SRCARCH)/sha256.h */
    #else
    #define sha256_blocks sha256_blocks_generic
    #endif

That patchset is targeting 6.17, though.  So we had to do this separate fix for
6.16 which has the odd sha256_choose_blocks() thing.

- Eric

Powered by blists - more mailing lists