From 8c67ac0f7f8863e60714f16513b4d420c764c093 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 9 Jun 2025 23:34:59 +0200 Subject: [PATCH] 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 --- ChangeLog.d/aesni_has_support.txt | 15 +++++++++++++++ library/aesni.c | 8 ++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 ChangeLog.d/aesni_has_support.txt diff --git a/ChangeLog.d/aesni_has_support.txt b/ChangeLog.d/aesni_has_support.txt new file mode 100644 index 0000000000..6ae0a56584 --- /dev/null +++ b/ChangeLog.d/aesni_has_support.txt @@ -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. diff --git a/library/aesni.c b/library/aesni.c index 4fc1cb918b..c51957f6ef 100644 --- a/library/aesni.c +++ b/library/aesni.c @@ -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