Skip to content

Commit 4f27e34

Browse files
committed
Merge bitcoin#728: Suppress a harmless variable-time optimization by clang in memczero
0199387 Add test for memczero() (Tim Ruffing) 52a0351 Suppress a harmless variable-time optimization by clang in memczero (Tim Ruffing) Pull request description: ACKs for top commit: jonasnick: ACK 0199387 Tree-SHA512: ed385f6e3909299b9979254bd0208a9d0c68caa9f57039e392aa0d5424ed8002c13f8c037bfbd2697c2f6b54af5731209eba9725c5e88be3ee3077c159d134e2
2 parents 8f78e20 + 0199387 commit 4f27e34

File tree

2 files changed

+26
-5
lines changed

2 files changed

+26
-5
lines changed

src/tests.c

+18
Original file line numberDiff line numberDiff line change
@@ -5166,6 +5166,21 @@ void run_ecdsa_openssl(void) {
51665166
# include "modules/recovery/tests_impl.h"
51675167
#endif
51685168

5169+
void run_memczero_test(void) {
5170+
unsigned char buf1[6] = {1, 2, 3, 4, 5, 6};
5171+
unsigned char buf2[sizeof(buf1)];
5172+
5173+
/* memczero(..., ..., 0) is a noop. */
5174+
memcpy(buf2, buf1, sizeof(buf1));
5175+
memczero(buf1, sizeof(buf1), 0);
5176+
CHECK(memcmp(buf1, buf2, sizeof(buf1)) == 0);
5177+
5178+
/* memczero(..., ..., 1) zeros the buffer. */
5179+
memset(buf2, 0, sizeof(buf2));
5180+
memczero(buf1, sizeof(buf1) , 1);
5181+
CHECK(memcmp(buf1, buf2, sizeof(buf1)) == 0);
5182+
}
5183+
51695184
int main(int argc, char **argv) {
51705185
unsigned char seed16[16] = {0};
51715186
unsigned char run32[32] = {0};
@@ -5299,6 +5314,9 @@ int main(int argc, char **argv) {
52995314
run_recovery_tests();
53005315
#endif
53015316

5317+
/* util tests */
5318+
run_memczero_test();
5319+
53025320
secp256k1_rand256(run32);
53035321
printf("random run = %02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x\n", run32[0], run32[1], run32[2], run32[3], run32[4], run32[5], run32[6], run32[7], run32[8], run32[9], run32[10], run32[11], run32[12], run32[13], run32[14], run32[15]);
53045322

src/util.h

+8-5
Original file line numberDiff line numberDiff line change
@@ -160,13 +160,16 @@ static SECP256K1_INLINE void *manual_alloc(void** prealloc_ptr, size_t alloc_siz
160160
SECP256K1_GNUC_EXT typedef unsigned __int128 uint128_t;
161161
#endif
162162

163-
/* Zero memory if flag == 1. Constant time. */
163+
/* Zero memory if flag == 1. Flag must be 0 or 1. Constant time. */
164164
static SECP256K1_INLINE void memczero(void *s, size_t len, int flag) {
165-
unsigned char *p;
166-
unsigned char mask = -(unsigned char)flag;
167-
p = (unsigned char *)s;
165+
unsigned char *p = (unsigned char *)s;
166+
/* Access flag with a volatile-qualified lvalue.
167+
This prevents clang from figuring out (after inlining) that flag can
168+
take only be 0 or 1, which leads to variable time code. */
169+
volatile int vflag = flag;
170+
unsigned char mask = -(unsigned char) vflag;
168171
while (len) {
169-
*p ^= *p & mask;
172+
*p &= ~mask;
170173
p++;
171174
len--;
172175
}

0 commit comments

Comments
 (0)