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

[Ard Biesheuvel]

(cc Tom)

On Tue, 31 Mar 2026, at 07:02, Eric Biggers wrote:
> [Added x86@xxxxxxxxxx and nikunj@xxxxxxx]
>
> 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.
>> 
...
> 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@xxxxxxxxxx/):
>
>     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.
>

AIUI, if we drop the IRQ dis/enable from this code, the generic path will be taken during early boot, but later invocations will use the accelerated implementations once they become available, right?

Mounting a timing attack requires accurate timing observations and a large number of samples, and it seems unlikely to me that a hostile VMM would be able to obtain those during the time window in question.