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>
---
 lib/crypto/aesgcm.c | 25 +++----------------------
 1 file changed, 3 insertions(+), 22 deletions(-)

diff --git a/lib/crypto/aesgcm.c b/lib/crypto/aesgcm.c
index 8c7e74d2d147..1da31e1f747d 100644
--- a/lib/crypto/aesgcm.c
+++ b/lib/crypto/aesgcm.c
@@ -7,29 +7,10 @@
 
 #include <crypto/gcm.h>
 #include <crypto/utils.h>
 #include <linux/export.h>
 #include <linux/module.h>
-#include <asm/irqflags.h>
-
-static void aesgcm_encrypt_block(const struct aes_enckey *key, void *dst,
-				 const void *src)
-{
-	unsigned long flags;
-
-	/*
-	 * In AES-GCM, both the GHASH key derivation and the CTR mode
-	 * encryption operate on known plaintext, making them susceptible to
-	 * timing attacks on the encryption key. The AES library already
-	 * mitigates this risk to some extent by pulling the entire S-box into
-	 * the caches before doing any substitutions, but this strategy is more
-	 * effective when running with interrupts disabled.
-	 */
-	local_irq_save(flags);
-	aes_encrypt(key, dst, src);
-	local_irq_restore(flags);
-}
 
 /**
  * aesgcm_expandkey - Expands the AES and GHASH keys for the AES-GCM key
  *		      schedule
  *
@@ -51,11 +32,11 @@ int aesgcm_expandkey(struct aesgcm_ctx *ctx, const u8 *key,
 	      aes_prepareenckey(&ctx->aes_key, key, keysize);
 	if (ret)
 		return ret;
 
 	ctx->authsize = authsize;
-	aesgcm_encrypt_block(&ctx->aes_key, h, h);
+	aes_encrypt(&ctx->aes_key, h, h);
 	ghash_preparekey(&ctx->ghash_key, h);
 	memzero_explicit(h, sizeof(h));
 	return 0;
 }
 EXPORT_SYMBOL(aesgcm_expandkey);
@@ -96,11 +77,11 @@ static void aesgcm_mac(const struct aesgcm_ctx *ctx, const u8 *src, int src_len,
 	ghash_update(&ghash, (const u8 *)&tail, sizeof(tail));
 
 	ghash_final(&ghash, ghash_out);
 
 	ctr[3] = cpu_to_be32(1);
-	aesgcm_encrypt_block(&ctx->aes_key, enc_ctr, ctr);
+	aes_encrypt(&ctx->aes_key, enc_ctr, (const u8 *)ctr);
 	crypto_xor_cpy(authtag, ghash_out, enc_ctr, ctx->authsize);
 
 	memzero_explicit(ghash_out, sizeof(ghash_out));
 	memzero_explicit(enc_ctr, sizeof(enc_ctr));
 }
@@ -118,11 +99,11 @@ static void aesgcm_crypt(const struct aesgcm_ctx *ctx, u8 *dst, const u8 *src,
 		 * inadvertent IV reuse, which must be avoided at all cost for
 		 * stream ciphers such as AES-CTR. Given the range of 'int
 		 * len', this cannot happen, so no explicit test is necessary.
 		 */
 		ctr[3] = cpu_to_be32(n++);
-		aesgcm_encrypt_block(&ctx->aes_key, buf, ctr);
+		aes_encrypt(&ctx->aes_key, buf, (const u8 *)ctr);
 		crypto_xor_cpy(dst, src, buf, min(len, AES_BLOCK_SIZE));
 
 		dst += AES_BLOCK_SIZE;
 		src += AES_BLOCK_SIZE;
 		len -= AES_BLOCK_SIZE;

base-commit: d2a68aba8505ce88b39c34ecb3b707c776af79d4
prerequisite-patch-id: bb75bceea1086ce63912baf959cd010cdd451208
-- 
2.53.0

[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

(cc Tom)

On Tue, 31 Mar 2026, at 07:02, Eric Biggers wrote:
> [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.
>> 
...
> 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.
>

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.

On Tue, Mar 31, 2026 at 09:05:23AM +0200, Ard Biesheuvel wrote:
> (cc Tom)
> 
> On Tue, 31 Mar 2026, at 07:02, Eric Biggers wrote:
> > [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.
> >> 
> ...
> > 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.
> >
> 
> 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?

Yes, that's correct.  The optimized code gets enabled by a
subsys_initcall.

> 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.

Seems plausible, since it looks like during early boot there is just one
call to each of aesgcm_expandkey(), aesgcm_encrypt(), and
aesgcm_decrypt().  Specifically during snp_secure_tsc_prepare().  Any
other AES-GCM operations happen later as calls from
drivers/virt/coco/sev-guest/sev-guest.c, which does not run that early.

A malicious VMM being able to inject interrupts arbitrarily is a bit
scary, though.

- Eric

On Tue, Mar 31, 2026 at 01:55:11PM -0700, Eric Biggers wrote:
> > 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?
> 
> Yes, that's correct.  The optimized code gets enabled by a
> subsys_initcall.

Also just to clarify, once the optimized crypto library code has been
enabled by the initcalls, it applies to all later function calls.  So
the library (e.g. the aesgcm_*() functions) doesn't have the problem
that the traditional crypto API (e.g. crypto_aead) has where unoptimized
code may continue to be used for an arbitrarily long time.

Anyway, I'll plan to apply this patch.  But it would be interesting to
hear from the x86 and SEV folks whether there is interest in making the
early AES-GCM operations in snp_secure_tsc_prepare() use the AES-NI and
PCLMULQDQ optimized code for better performance and side-channel
resistance.

- Eric