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.
This commit is contained in:
Tim Ruffing 2020-03-25 16:04:49 +01:00
parent 8f78e208ad
commit 52a03512c1

View File

@ -162,11 +162,14 @@ SECP256K1_GNUC_EXT typedef unsigned __int128 uint128_t;
/* Zero memory if flag == 1. Constant time. */ /* Zero memory if flag == 1. Constant time. */
static SECP256K1_INLINE void memczero(void *s, size_t len, int flag) { static SECP256K1_INLINE void memczero(void *s, size_t len, int flag) {
unsigned char *p; unsigned char *p = (unsigned char *)s;
unsigned char mask = -(unsigned char)flag; /* Access flag with a volatile-qualified lvalue.
p = (unsigned char *)s; 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 vflag = flag;
unsigned char mask = -(unsigned char) vflag;
while (len) { while (len) {
*p ^= *p & mask; *p &= ~mask;
p++; p++;
len--; len--;
} }