From 5d1aa7fbc5730482e1dbedc209c540d136ab1f7f Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Wed, 25 Mar 2020 16:04:49 +0100 Subject: [PATCH] Suppress a harmless variable-time optimization by clang in memczero This has been not been caught by the new constant-time tests because valgrind currently gives us a zero exit code even if finds errors, see https://github.com/bitcoin-core/secp256k1/pull/723#discussion_r388246806 . This commit also simplifies the arithmetic in memczero. Note that the timing leak here was the bit whether a secret key was out of range. This leak is harmless and not exploitable. It is just our overcautious practice to prefer constant-time code even here. --- src/util.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/util.h b/src/util.h index 2f96e2fc27..5f37098b03 100644 --- a/src/util.h +++ b/src/util.h @@ -163,10 +163,14 @@ SECP256K1_GNUC_EXT typedef unsigned __int128 uint128_t; /* Zero memory if flag == 1. Constant time. */ static SECP256K1_INLINE void memczero(void *s, size_t len, int flag) { unsigned char *p; - unsigned char mask = -(unsigned char)flag; + /* Access flag with a volatile-qualified lvalue. + This prevents clang from figuring out (after inlining) that flag can + take only be 0 or 1, which leads to variable time code. */ + volatile int *flagp = &flag; + unsigned char mask = -(unsigned char) *flagp; p = (unsigned char *)s; while (len) { - *p ^= *p & mask; + *p &= ~mask; p++; len--; }