From 8c67ac0f7f8863e60714f16513b4d420c764c093 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 9 Jun 2025 23:34:59 +0200 Subject: [PATCH 1/3] 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 From f5db3e9436cc004abcc22de3b35ee02405d2ffb7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 11 Jun 2025 10:45:41 +0200 Subject: [PATCH 2/3] Note that GCM is also impacted Signed-off-by: Gilles Peskine --- ChangeLog.d/aesni_has_support.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog.d/aesni_has_support.txt b/ChangeLog.d/aesni_has_support.txt index 6ae0a56584..26b7c2c59b 100644 --- a/ChangeLog.d/aesni_has_support.txt +++ b/ChangeLog.d/aesni_has_support.txt @@ -13,3 +13,5 @@ Security the adversary to conduct timing attacks and potentially recover the key. In particular, this attacker model may be possible against an SGX enclave. + The same vulnerability affects GCM acceleration, which could allow + a similarly powerful adversary to craft GCM forgeries. From 853cfbdcedece871c8f5ac4bfd239d2d7b8b7899 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 12 Jun 2025 18:30:45 +0200 Subject: [PATCH 3/3] Add a note about processor memory reordering Signed-off-by: Gilles Peskine --- library/aesni.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/library/aesni.c b/library/aesni.c index c51957f6ef..2857068244 100644 --- a/library/aesni.c +++ b/library/aesni.c @@ -51,6 +51,13 @@ int mbedtls_aesni_has_support(unsigned int what) /* 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 + * + * Note that we may also be worried about memory access reordering, + * but fortunately the x86 memory model is not too wild: stores + * from the same thread are observed consistently by other threads. + * (See example 8-1 in Sewell et al., "x86-TSO: A Rigorous and Usable + * Programmer’s Model for x86 Multiprocessors", CACM, 2010, + * https://www.cl.cam.ac.uk/~pes20/weakmemory/cacm.pdf) */ static volatile int done = 0; static volatile unsigned int c = 0;