Skip to content

Commit

Permalink
Don't use reserved identifiers memczero and benchmark_verify_t
Browse files Browse the repository at this point in the history
As identified in bitcoin#829 and bitcoin#833. Fixes bitcoin#829.

Since we touch this anyway, this commit additionally makes the
identifiers in the benchmark files a little bit more consistent.
  • Loading branch information
real-or-random committed Oct 20, 2020
1 parent c6b6b8f commit e89278f
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 22 deletions.
8 changes: 4 additions & 4 deletions src/bench_sign.c
Expand Up @@ -12,11 +12,11 @@ typedef struct {
secp256k1_context* ctx;
unsigned char msg[32];
unsigned char key[32];
} bench_sign;
} bench_sign_data;

static void bench_sign_setup(void* arg) {
int i;
bench_sign *data = (bench_sign*)arg;
bench_sign_data *data = (bench_sign_data*)arg;

for (i = 0; i < 32; i++) {
data->msg[i] = i + 1;
Expand All @@ -28,7 +28,7 @@ static void bench_sign_setup(void* arg) {

static void bench_sign_run(void* arg, int iters) {
int i;
bench_sign *data = (bench_sign*)arg;
bench_sign_data *data = (bench_sign_data*)arg;

unsigned char sig[74];
for (i = 0; i < iters; i++) {
Expand All @@ -45,7 +45,7 @@ static void bench_sign_run(void* arg, int iters) {
}

int main(void) {
bench_sign data;
bench_sign_data data;

int iters = get_iters(20000);

Expand Down
16 changes: 8 additions & 8 deletions src/bench_verify.c
Expand Up @@ -29,11 +29,11 @@ typedef struct {
#ifdef ENABLE_OPENSSL_TESTS
EC_GROUP* ec_group;
#endif
} benchmark_verify_t;
} bench_verify_data;

static void benchmark_verify(void* arg, int iters) {
static void bench_verify(void* arg, int iters) {
int i;
benchmark_verify_t* data = (benchmark_verify_t*)arg;
bench_verify_data* data = (bench_verify_data*)arg;

for (i = 0; i < iters; i++) {
secp256k1_pubkey pubkey;
Expand All @@ -51,9 +51,9 @@ static void benchmark_verify(void* arg, int iters) {
}

#ifdef ENABLE_OPENSSL_TESTS
static void benchmark_verify_openssl(void* arg, int iters) {
static void bench_verify_openssl(void* arg, int iters) {
int i;
benchmark_verify_t* data = (benchmark_verify_t*)arg;
bench_verify_data* data = (bench_verify_data*)arg;

for (i = 0; i < iters; i++) {
data->sig[data->siglen - 1] ^= (i & 0xFF);
Expand Down Expand Up @@ -84,7 +84,7 @@ int main(void) {
int i;
secp256k1_pubkey pubkey;
secp256k1_ecdsa_signature sig;
benchmark_verify_t data;
bench_verify_data data;

int iters = get_iters(20000);

Expand All @@ -103,10 +103,10 @@ int main(void) {
data.pubkeylen = 33;
CHECK(secp256k1_ec_pubkey_serialize(data.ctx, data.pubkey, &data.pubkeylen, &pubkey, SECP256K1_EC_COMPRESSED) == 1);

run_benchmark("ecdsa_verify", benchmark_verify, NULL, NULL, &data, 10, iters);
run_benchmark("ecdsa_verify", bench_verify, NULL, NULL, &data, 10, iters);
#ifdef ENABLE_OPENSSL_TESTS
data.ec_group = EC_GROUP_new_by_curve_name(NID_secp256k1);
run_benchmark("ecdsa_verify_openssl", benchmark_verify_openssl, NULL, NULL, &data, 10, iters);
run_benchmark("ecdsa_verify_openssl", bench_verify_openssl, NULL, NULL, &data, 10, iters);
EC_GROUP_free(data.ec_group);
#endif

Expand Down
2 changes: 1 addition & 1 deletion src/modules/extrakeys/main_impl.h
Expand Up @@ -180,7 +180,7 @@ int secp256k1_keypair_create(const secp256k1_context* ctx, secp256k1_keypair *ke

ret = secp256k1_ec_pubkey_create_helper(&ctx->ecmult_gen_ctx, &sk, &pk, seckey32);
secp256k1_keypair_save(keypair, &sk, &pk);
memczero(keypair, sizeof(*keypair), !ret);
secp256k1_memczero(keypair, sizeof(*keypair), !ret);

secp256k1_scalar_clear(&sk);
return ret;
Expand Down
2 changes: 1 addition & 1 deletion src/modules/schnorrsig/main_impl.h
Expand Up @@ -179,7 +179,7 @@ int secp256k1_schnorrsig_sign(const secp256k1_context* ctx, unsigned char *sig64
secp256k1_scalar_add(&e, &e, &k);
secp256k1_scalar_get_b32(&sig64[32], &e);

memczero(sig64, 64, !ret);
secp256k1_memczero(sig64, 64, !ret);
secp256k1_scalar_clear(&k);
secp256k1_scalar_clear(&sk);
memset(seckey, 0, sizeof(seckey));
Expand Down
2 changes: 1 addition & 1 deletion src/secp256k1.c
Expand Up @@ -580,7 +580,7 @@ int secp256k1_ec_pubkey_create(const secp256k1_context* ctx, secp256k1_pubkey *p

ret = secp256k1_ec_pubkey_create_helper(&ctx->ecmult_gen_ctx, &seckey_scalar, &p, seckey);
secp256k1_pubkey_save(pubkey, &p);
memczero(pubkey, sizeof(*pubkey), !ret);
secp256k1_memczero(pubkey, sizeof(*pubkey), !ret);

secp256k1_scalar_clear(&seckey_scalar);
return ret;
Expand Down
12 changes: 6 additions & 6 deletions src/tests.c
Expand Up @@ -5444,18 +5444,18 @@ void run_ecdsa_openssl(void) {
# include "modules/schnorrsig/tests_impl.h"
#endif

void run_memczero_test(void) {
void run_secp256k1_memczero_test(void) {
unsigned char buf1[6] = {1, 2, 3, 4, 5, 6};
unsigned char buf2[sizeof(buf1)];

/* memczero(..., ..., 0) is a noop. */
/* secp256k1_memczero(..., ..., 0) is a noop. */
memcpy(buf2, buf1, sizeof(buf1));
memczero(buf1, sizeof(buf1), 0);
secp256k1_memczero(buf1, sizeof(buf1), 0);
CHECK(secp256k1_memcmp_var(buf1, buf2, sizeof(buf1)) == 0);

/* memczero(..., ..., 1) zeros the buffer. */
/* secp256k1_memczero(..., ..., 1) zeros the buffer. */
memset(buf2, 0, sizeof(buf2));
memczero(buf1, sizeof(buf1) , 1);
secp256k1_memczero(buf1, sizeof(buf1) , 1);
CHECK(secp256k1_memcmp_var(buf1, buf2, sizeof(buf1)) == 0);
}

Expand Down Expand Up @@ -5723,7 +5723,7 @@ int main(int argc, char **argv) {
#endif

/* util tests */
run_memczero_test();
run_secp256k1_memczero_test();

run_cmov_tests();

Expand Down
2 changes: 1 addition & 1 deletion src/util.h
Expand Up @@ -202,7 +202,7 @@ static SECP256K1_INLINE void *manual_alloc(void** prealloc_ptr, size_t alloc_siz
#endif

/* Zero memory if flag == 1. Flag must be 0 or 1. Constant time. */
static SECP256K1_INLINE void memczero(void *s, size_t len, int flag) {
static SECP256K1_INLINE void secp256k1_memczero(void *s, size_t len, int flag) {
unsigned char *p = (unsigned char *)s;
/* Access flag with a volatile-qualified lvalue.
This prevents clang from figuring out (after inlining) that flag can
Expand Down

0 comments on commit e89278f

Please sign in to comment.