On Fri, Jun 27, 2025 at 05:54:05PM -0700, Linus Torvalds wrote: > On Fri, 27 Jun 2025 at 11:15, Eric Biggers <ebiggers@xxxxxxxxxx> 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@xxxxxxxxxx/ 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