All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
	Ard Biesheuvel <ardb@kernel.org>,
	"Jason A. Donenfeld" <Jason@zx2c4.com>,
	Arnd Bergmann <arnd@arndb.de>
Subject: Re: [GIT PULL] Crypto library fix for v6.16-rc4
Date: Sat, 28 Jun 2025 01:18:34 +0000	[thread overview]
Message-ID: <20250628011834.GA1246405@google.com> (raw)
In-Reply-To: <CAHk-=wiT=UUcgCVVo5ui_2Xb9fdg4JrPK=ZqpPxDhCgq9vynDg@mail.gmail.com>

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

  reply	other threads:[~2025-06-28  1:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-27 18:14 [GIT PULL] Crypto library fix for v6.16-rc4 Eric Biggers
2025-06-28  0:54 ` Linus Torvalds
2025-06-28  1:18   ` Eric Biggers [this message]
2025-06-28  2:55     ` Linus Torvalds
2025-06-28  1:02 ` pr-tracker-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250628011834.GA1246405@google.com \
    --to=ebiggers@kernel.org \
    --cc=Jason@zx2c4.com \
    --cc=ardb@kernel.org \
    --cc=arnd@arndb.de \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.