Re: Banning crypto in hardirq context (was: [PATCH v2 0/9] crypto: x86 - stop using the SIMD helper)

[Eric Biggers <ebiggers@kernel.org>]

On Thu, Apr 03, 2025 at 11:42:34AM +0800, Herbert Xu wrote:
> On Wed, Apr 02, 2025 at 08:20:08PM -0700, Eric Biggers wrote:
> >
> > Also, riscv has scalar AES instructions.  (They aren't used by the kernel yet,
> > but they could be.  The CRC code already uses scalar carryless multiplication.)
> 
> It still doesn't mean that it's a good idea to use AES in a
> hard IRQ handler, especially if the code is meant to be portable.
> 
> > Also, as I said already, x86 does support SIMD instructions in hardirq context
> > in some cases.  Whether anyone actually uses that, I don't know, but it is
> > explicitly supported.  Check out irq_fpu_usable().
> 
> This is more of an accident than some deliberate strategy of
> supporting FPU usage in hard IRQs.  This test was initially
> added for aesni:
> 
> commit 54b6a1bd5364aca95cd6ffae00f2b64c6511122c
> Author: Ying Huang <huang.ying.caritas@gmail.com>
> Date:   Sun Jan 18 16:28:34 2009 +1100
> 
>     crypto: aes-ni - Add support to Intel AES-NI instructions for x86_64 platform
> 
> It was then improved by:
> 
> Author: Linus Torvalds <torvalds@linux-foundation.org>
> Date:   Mon Feb 13 13:56:14 2012 -0800
> 
>     i387: make irq_fpu_usable() tests more robust
>     
>     Some code - especially the crypto layer - wants to use the x86
>     FP/MMX/AVX register set in what may be interrupt (typically softirq)
>     context.
> 
> At no point was there any intention of using this in a hardirq
> context.
> 
> Until such a time when you have a valid application for using
> lib/crypto code in a hardirq context, I don't think we should
> be supporting that at the expense of real users who are in
> process/softirq context only.

Whatever.  We agree that "crypto in hardirq" is not a good idea in general.  I'm
just pointing out that there are certain cases, like SipHash used in a hash
table, where it easily could happen and would be fine.  And all the shash and
crypto library functions currently work in any context, unlike e.g. skcipher and
aead which do not.  You seem to be trying to claim that it was never supported,
but that is incorrect.  Making it unsupported would be a change that needs to be
properly documented (the functions would no longer be simply "Any context")
*and* have proper debug assertions added to enforce it and prevent usage errors.
But in a lot of cases there is also no reason to even add that restriction.  I'm
not sure why you're so eager to make the library functions harder to use.

- Eric