Skip to content

Commit

Permalink
Suppress a harmless variable-time optimization by clang in memczero
Browse files Browse the repository at this point in the history
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
#723 (comment) .

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.
  • Loading branch information
real-or-random committed Mar 25, 2020
1 parent 8f78e20 commit 5d1aa7f
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -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--;
}
Expand Down

0 comments on commit 5d1aa7f

Please sign in to comment.