Fix race condition in mbedtls_aesni_has_support

Fix a race condition in `mbedtls_aes_ni_has_support()` with some compilers.
A compiler could hoist the assignment `done = 1` above the assignment to `c`,
in which case if two threads call `mbedtls_aes_ni_has_support()` at almost
the same time, they could be interleaved as follows:

    Initially: done = 0, c = 0

    thread A                thread B
    if (!done)
      done = 1;                                     # hoisted
                            if (!done)
                                return c & what;    # wrong!
      c = cpuid();
    return c & what

This would lead to thread B using software AES even though AESNI was
available. This is a very minor performance bug. But also, given a very
powerful adversary who can block thread A indefinitely (which may be
possible when attacking an SGX enclave), thread B could use software AES for
a long time, opening the way to a timing side channel attack.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This commit is contained in:
Gilles Peskine
2025-06-09 23:34:59 +02:00
parent 3f82706cb7
commit 8c67ac0f7f
2 changed files with 21 additions and 2 deletions

View File

@@ -0,0 +1,15 @@
Bugfix
* Fix a race condition on x86/amd64 platforms in AESNI support detection
that could lead to using software AES in some threads at the very
beginning of a multithreaded program. Reported by Solar Designer.
Fixes #9840.
Security
* On x86/amd64 platforms, with some compilers, when the library is
compiled with support for both AESNI and software AES and AESNI is
available in hardware, an adversary with fine control over which
threads make progress in a multithreaded program could force software
AES to be used for some time when the program starts. This could allow
the adversary to conduct timing attacks and potentially recover the
key. In particular, this attacker model may be possible against an SGX
enclave.

View File

@@ -48,8 +48,12 @@
*/
int mbedtls_aesni_has_support(unsigned int what)
{
static int done = 0;
static unsigned int c = 0;
/* To avoid a race condition, tell the compiler that the assignment
* `done = 1` and the assignment to `c` may not be reordered.
* https://github.com/Mbed-TLS/mbedtls/issues/9840
*/
static volatile int done = 0;
static volatile unsigned int c = 0;
if (!done) {
#if MBEDTLS_AESNI_HAVE_CODE == 2