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>
diff --git a/ChangeLog.d/aesni_has_support.txt b/ChangeLog.d/aesni_has_support.txt
new file mode 100644
index 0000000..6ae0a56
--- /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 4fc1cb9..c51957f 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