Re: [PATCH] lib/crypto: aesgcm: Don't disable IRQs during AES block encryption

[Eric Biggers <ebiggers@kernel.org>]

[Added x86@kernel.org and nikunj@amd.com]

On Mon, Mar 30, 2026 at 07:44:30PM -0700, Eric Biggers wrote:
> aes_encrypt() now uses AES instructions when available instead of always
> using table-based code.  AES instructions are constant-time and don't
> benefit from disabling IRQs as a constant-time hardening measure.
> 
> In fact, on two architectures (arm and riscv) disabling IRQs is
> counterproductive because it prevents the AES instructions from being
> used.  (See the may_use_simd() implementation on those architectures.)
> 
> Therefore, let's remove the IRQ disabling/enabling and leave the choice
> of constant-time hardening measures to the AES library code.
> 
> Note that currently the arm table-based AES code (which runs on arm
> kernels that don't have ARMv8 CE) disables IRQs, while the generic
> table-based AES code does not.  So this does technically regress in
> constant-time hardening when that generic code is used.  But as
> discussed in commit a22fd0e3c495 ("lib/crypto: aes: Introduce improved
> AES library") I think just leaving IRQs enabled is the right choice.
> Disabling them is slow and can cause problems, and AES instructions
> (which modern CPUs have) solve the problem in a much better way anyway.
> 
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>

I just noticed the rationale in the patch series that originally added
lib/crypto/aesgcm.c in 2022
(https://lore.kernel.org/all/20221103192259.2229-1-ardb@kernel.org/):

    Provide a generic library implementation of AES-GCM which can be
    used really early during boot, e.g., to communicate with the
    security coprocessor on SEV-SNP virtual machines to bring up
    secondary cores.  This is needed because the crypto API is not
    available yet this early.

    We cannot rely on special instructions for AES or polynomial
    multiplication, which are arch specific and rely on in-kernel SIMD
    infrastructure. Instead, add a generic C implementation that
    combines the existing C implementations of AES and multiplication in
    GF(2^128).

    To reduce the risk of forgery attacks, replace data dependent table
    lookups and conditional branches in the used gf128mul routine with
    constant-time equivalents. The AES library has already been
    robustified to some extent to prevent known-plaintext timing attacks
    on the key, but we call it with interrupts disabled to make it a bit
    more robust. (Note that in SEV-SNP context, the VMM is untrusted,
    and is able to inject interrupts arbitrarily, and potentially
    maliciously.)

So, the user of AES-GCM in arch/x86/coco/sev/ is a bit special.  It runs
super early, before the crypto library initcalls have run and enabled
the use of AES-NI and PCLMULQDQ optimized routines.  And apparently it
really needs protection from timing attacks, as well.

I think this patch is still the way to go, but it does slightly weaken
the protection from timing attacks for super early users like this.  So
I think we'll likely want to do something else as well.  Either:

- Disable IRQs in the callers in arch/x86/coco/sev/.

- Or, enable the AES-NI and PCLMULQDQ optimized crypto library routines
  earlier on x86, so that they will be used in this case.  Specifically,
  enable them in arch_cpu_finalize_init() between fpu__init_cpu() and
  mem_encrypt_init().

I'd prefer the latter.  The dedicated instructions are the proper way to
get data and key-independent timing for AES-GCM.  It's much less clear
that the generic C code has data and key-independent timing, even if
it's run with IRQs disabled.

Any thoughts?

- Eric