From c4fd5dab451ca3983e50b5d45e0b0ca109486ddc Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Mon, 22 Oct 2018 16:25:26 +0200 Subject: [PATCH] Switch to a single malloc call --- include/secp256k1.h | 6 ++++++ src/ecmult.h | 5 ++--- src/ecmult_gen.h | 5 ++--- src/ecmult_gen_impl.h | 29 ++++++++++--------------- src/ecmult_impl.h | 32 ++++++++++------------------ src/gen_context.c | 13 ++++++++---- src/secp256k1.c | 49 ++++++++++++++++++++++++++++++++----------- src/tests.c | 4 ---- 8 files changed, 78 insertions(+), 65 deletions(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index 43af09c330de4..b3b5d0e5591d2 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -187,6 +187,9 @@ typedef int (*secp256k1_nonce_function)( SECP256K1_API extern const secp256k1_context *secp256k1_context_no_precomp; /** Create a secp256k1 context object. + * + * This function uses malloc to allocate memory. It is guaranteed that malloc is + * called at most once for every call of this function. * * Returns: a newly created context object. * In: flags: which parts of the context to initialize. @@ -198,6 +201,9 @@ SECP256K1_API secp256k1_context* secp256k1_context_create( ) SECP256K1_WARN_UNUSED_RESULT; /** Copies a secp256k1 context object. + * + * This function uses malloc to allocate memory. It is guaranteed that malloc is + * called at most once for every call of this function. * * Returns: a newly created context object. * Args: ctx: an existing context to copy (cannot be NULL) diff --git a/src/ecmult.h b/src/ecmult.h index 6ac78bc3e6ca7..29fe0b082321f 100644 --- a/src/ecmult.h +++ b/src/ecmult.h @@ -22,9 +22,8 @@ typedef struct { static const size_t SECP256K1_ECMULT_CONTEXT_PREALLOCATED_SIZE; static void secp256k1_ecmult_context_init(secp256k1_ecmult_context *ctx); -static void secp256k1_ecmult_context_build(secp256k1_ecmult_context *ctx, const secp256k1_callback *cb); -static void secp256k1_ecmult_context_clone(secp256k1_ecmult_context *dst, - const secp256k1_ecmult_context *src, const secp256k1_callback *cb); +static void secp256k1_ecmult_context_build(secp256k1_ecmult_context *ctx, void **prealloc); +static void secp256k1_ecmult_context_finalize_memcpy(secp256k1_ecmult_context *dst, const secp256k1_ecmult_context *src); static void secp256k1_ecmult_context_clear(secp256k1_ecmult_context *ctx); static int secp256k1_ecmult_context_is_built(const secp256k1_ecmult_context *ctx); diff --git a/src/ecmult_gen.h b/src/ecmult_gen.h index 0e23bc1095a7c..b136e94632ddf 100644 --- a/src/ecmult_gen.h +++ b/src/ecmult_gen.h @@ -30,9 +30,8 @@ typedef struct { static const size_t SECP256K1_ECMULT_GEN_CONTEXT_PREALLOCATED_SIZE; static void secp256k1_ecmult_gen_context_init(secp256k1_ecmult_gen_context* ctx); -static void secp256k1_ecmult_gen_context_build(secp256k1_ecmult_gen_context* ctx, const secp256k1_callback* cb); -static void secp256k1_ecmult_gen_context_clone(secp256k1_ecmult_gen_context *dst, - const secp256k1_ecmult_gen_context* src, const secp256k1_callback* cb); +static void secp256k1_ecmult_gen_context_build(secp256k1_ecmult_gen_context* ctx, void **prealloc); +static void secp256k1_ecmult_gen_context_finalize_memcpy(secp256k1_ecmult_gen_context *dst, const secp256k1_ecmult_gen_context* src); static void secp256k1_ecmult_gen_context_clear(secp256k1_ecmult_gen_context* ctx); static int secp256k1_ecmult_gen_context_is_built(const secp256k1_ecmult_gen_context* ctx); diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index 1188a21367683..f818d45c298b4 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -26,19 +26,21 @@ static void secp256k1_ecmult_gen_context_init(secp256k1_ecmult_gen_context *ctx) ctx->prec = NULL; } -static void secp256k1_ecmult_gen_context_build(secp256k1_ecmult_gen_context *ctx, const secp256k1_callback* cb) { +static void secp256k1_ecmult_gen_context_build(secp256k1_ecmult_gen_context *ctx, void **prealloc) { #ifndef USE_ECMULT_STATIC_PRECOMPUTATION secp256k1_ge prec[1024]; secp256k1_gej gj; secp256k1_gej nums_gej; int i, j; + size_t const prealloc_size = SECP256K1_ECMULT_GEN_CONTEXT_PREALLOCATED_SIZE; + void* const base = *prealloc; #endif if (ctx->prec != NULL) { return; } #ifndef USE_ECMULT_STATIC_PRECOMPUTATION - ctx->prec = (secp256k1_ge_storage (*)[64][16])checked_malloc(cb, sizeof(*ctx->prec)); + ctx->prec = (secp256k1_ge_storage (*)[64][16])manual_alloc(prealloc, prealloc_size, base, prealloc_size); /* get the generator */ secp256k1_gej_set_ge(&gj, &secp256k1_ge_const_g); @@ -93,7 +95,7 @@ static void secp256k1_ecmult_gen_context_build(secp256k1_ecmult_gen_context *ctx } } #else - (void)cb; + (void)prealloc; ctx->prec = (secp256k1_ge_storage (*)[64][16])secp256k1_ecmult_static_context; #endif secp256k1_ecmult_gen_blind(ctx, NULL); @@ -103,27 +105,18 @@ static int secp256k1_ecmult_gen_context_is_built(const secp256k1_ecmult_gen_cont return ctx->prec != NULL; } -static void secp256k1_ecmult_gen_context_clone(secp256k1_ecmult_gen_context *dst, - const secp256k1_ecmult_gen_context *src, const secp256k1_callback* cb) { - if (src->prec == NULL) { - dst->prec = NULL; - } else { +static void secp256k1_ecmult_gen_context_finalize_memcpy(secp256k1_ecmult_gen_context *dst, const secp256k1_ecmult_gen_context *src) { #ifndef USE_ECMULT_STATIC_PRECOMPUTATION - dst->prec = (secp256k1_ge_storage (*)[64][16])checked_malloc(cb, sizeof(*dst->prec)); - memcpy(dst->prec, src->prec, sizeof(*dst->prec)); + if (src->prec != NULL) { + /* We cast to void* first to suppress a -Wcast-align warning. */ + dst->prec = (secp256k1_ge_storage (*)[64][16])(void*)((unsigned char*)dst + ((unsigned char*)src->prec - (unsigned char*)src)); + } #else - (void)cb; - dst->prec = src->prec; + (void)dst, (void)src; #endif - dst->initial = src->initial; - dst->blind = src->blind; - } } static void secp256k1_ecmult_gen_context_clear(secp256k1_ecmult_gen_context *ctx) { -#ifndef USE_ECMULT_STATIC_PRECOMPUTATION - free(ctx->prec); -#endif secp256k1_scalar_clear(&ctx->blind); secp256k1_gej_clear(&ctx->initial); ctx->prec = NULL; diff --git a/src/ecmult_impl.h b/src/ecmult_impl.h index 70db458406c92..b60b1f84e3e79 100644 --- a/src/ecmult_impl.h +++ b/src/ecmult_impl.h @@ -325,8 +325,10 @@ static void secp256k1_ecmult_context_init(secp256k1_ecmult_context *ctx) { #endif } -static void secp256k1_ecmult_context_build(secp256k1_ecmult_context *ctx, const secp256k1_callback *cb) { +static void secp256k1_ecmult_context_build(secp256k1_ecmult_context *ctx, void **prealloc) { secp256k1_gej gj; + void* const base = *prealloc; + size_t const prealloc_size = SECP256K1_ECMULT_CONTEXT_PREALLOCATED_SIZE; if (ctx->pre_g != NULL) { return; @@ -339,7 +341,7 @@ static void secp256k1_ecmult_context_build(secp256k1_ecmult_context *ctx, const size_t size = sizeof((*ctx->pre_g)[0]) * ((size_t)ECMULT_TABLE_SIZE(WINDOW_G)); /* check for overflow */ VERIFY_CHECK(size / sizeof((*ctx->pre_g)[0]) == ((size_t)ECMULT_TABLE_SIZE(WINDOW_G))); - ctx->pre_g = (secp256k1_ge_storage (*)[])checked_malloc(cb, size); + ctx->pre_g = (secp256k1_ge_storage (*)[])manual_alloc(prealloc, sizeof((*ctx->pre_g)[0]) * ECMULT_TABLE_SIZE(WINDOW_G), base, prealloc_size); } /* precompute the tables with odd multiples */ @@ -353,7 +355,7 @@ static void secp256k1_ecmult_context_build(secp256k1_ecmult_context *ctx, const size_t size = sizeof((*ctx->pre_g_128)[0]) * ((size_t) ECMULT_TABLE_SIZE(WINDOW_G)); /* check for overflow */ VERIFY_CHECK(size / sizeof((*ctx->pre_g_128)[0]) == ((size_t)ECMULT_TABLE_SIZE(WINDOW_G))); - ctx->pre_g_128 = (secp256k1_ge_storage (*)[])checked_malloc(cb, size); + ctx->pre_g_128 = (secp256k1_ge_storage (*)[])manual_alloc(prealloc, sizeof((*ctx->pre_g_128)[0]) * ECMULT_TABLE_SIZE(WINDOW_G), base, prealloc_size); /* calculate 2^128*generator */ g_128j = gj; @@ -365,22 +367,14 @@ static void secp256k1_ecmult_context_build(secp256k1_ecmult_context *ctx, const #endif } -static void secp256k1_ecmult_context_clone(secp256k1_ecmult_context *dst, - const secp256k1_ecmult_context *src, const secp256k1_callback *cb) { - if (src->pre_g == NULL) { - dst->pre_g = NULL; - } else { - size_t size = sizeof((*dst->pre_g)[0]) * ((size_t)ECMULT_TABLE_SIZE(WINDOW_G)); - dst->pre_g = (secp256k1_ge_storage (*)[])checked_malloc(cb, size); - memcpy(dst->pre_g, src->pre_g, size); +static void secp256k1_ecmult_context_finalize_memcpy(secp256k1_ecmult_context *dst, const secp256k1_ecmult_context *src) { + if (src->pre_g != NULL) { + /* We cast to void* first to suppress a -Wcast-align warning. */ + dst->pre_g = (secp256k1_ge_storage (*)[])(void*)((unsigned char*)dst + ((unsigned char*)(src->pre_g) - (unsigned char*)src)); } #ifdef USE_ENDOMORPHISM - if (src->pre_g_128 == NULL) { - dst->pre_g_128 = NULL; - } else { - size_t size = sizeof((*dst->pre_g_128)[0]) * ((size_t)ECMULT_TABLE_SIZE(WINDOW_G)); - dst->pre_g_128 = (secp256k1_ge_storage (*)[])checked_malloc(cb, size); - memcpy(dst->pre_g_128, src->pre_g_128, size); + if (src->pre_g_128 != NULL) { + dst->pre_g_128 = (secp256k1_ge_storage (*)[])(void*)((unsigned char*)dst + ((unsigned char*)(src->pre_g_128) - (unsigned char*)src)); } #endif } @@ -390,10 +384,6 @@ static int secp256k1_ecmult_context_is_built(const secp256k1_ecmult_context *ctx } static void secp256k1_ecmult_context_clear(secp256k1_ecmult_context *ctx) { - free(ctx->pre_g); -#ifdef USE_ENDOMORPHISM - free(ctx->pre_g_128); -#endif secp256k1_ecmult_context_init(ctx); } diff --git a/src/gen_context.c b/src/gen_context.c index 87d296ebf0e2c..82c605c5d4f77 100644 --- a/src/gen_context.c +++ b/src/gen_context.c @@ -8,6 +8,7 @@ #include "basic-config.h" #include "include/secp256k1.h" +#include "util.h" #include "field_impl.h" #include "scalar_impl.h" #include "group_impl.h" @@ -26,6 +27,7 @@ static const secp256k1_callback default_error_callback = { int main(int argc, char **argv) { secp256k1_ecmult_gen_context ctx; + void *prealloc, *base; int inner; int outer; FILE* fp; @@ -38,15 +40,17 @@ int main(int argc, char **argv) { fprintf(stderr, "Could not open src/ecmult_static_context.h for writing!\n"); return -1; } - + fprintf(fp, "#ifndef _SECP256K1_ECMULT_STATIC_CONTEXT_\n"); fprintf(fp, "#define _SECP256K1_ECMULT_STATIC_CONTEXT_\n"); fprintf(fp, "#include \"src/group.h\"\n"); fprintf(fp, "#define SC SECP256K1_GE_STORAGE_CONST\n"); fprintf(fp, "static const secp256k1_ge_storage secp256k1_ecmult_static_context[64][16] = {\n"); + base = checked_malloc(&default_error_callback, SECP256K1_ECMULT_GEN_CONTEXT_PREALLOCATED_SIZE); + prealloc = base; secp256k1_ecmult_gen_context_init(&ctx); - secp256k1_ecmult_gen_context_build(&ctx, &default_error_callback); + secp256k1_ecmult_gen_context_build(&ctx, &prealloc); for(outer = 0; outer != 64; outer++) { fprintf(fp,"{\n"); for(inner = 0; inner != 16; inner++) { @@ -65,10 +69,11 @@ int main(int argc, char **argv) { } fprintf(fp,"};\n"); secp256k1_ecmult_gen_context_clear(&ctx); - + free(base); + fprintf(fp, "#undef SC\n"); fprintf(fp, "#endif\n"); fclose(fp); - + return 0; } diff --git a/src/secp256k1.c b/src/secp256k1.c index fe936c53193d9..9055f7b6cf06a 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -82,15 +82,17 @@ size_t secp256k1_context_preallocated_size(unsigned int flags) { return ret; } -secp256k1_context* secp256k1_context_create(unsigned int flags) { - secp256k1_context* ret = (secp256k1_context*)checked_malloc(&default_error_callback, sizeof(secp256k1_context)); +secp256k1_context* secp256k1_context_preallocated_create(void* prealloc, unsigned int flags) { + void* const base = prealloc; + size_t prealloc_size = secp256k1_context_preallocated_size(flags); + secp256k1_context* ret = (secp256k1_context*)manual_alloc(&prealloc, sizeof(secp256k1_context), base, prealloc_size); + ret->illegal_callback = default_illegal_callback; ret->error_callback = default_error_callback; if (EXPECT((flags & SECP256K1_FLAGS_TYPE_MASK) != SECP256K1_FLAGS_TYPE_CONTEXT, 0)) { secp256k1_callback_call(&ret->illegal_callback, "Invalid flags"); - free(ret); return NULL; } @@ -98,30 +100,53 @@ secp256k1_context* secp256k1_context_create(unsigned int flags) { secp256k1_ecmult_gen_context_init(&ret->ecmult_gen_ctx); if (flags & SECP256K1_FLAGS_BIT_CONTEXT_SIGN) { - secp256k1_ecmult_gen_context_build(&ret->ecmult_gen_ctx, &ret->error_callback); + secp256k1_ecmult_gen_context_build(&ret->ecmult_gen_ctx, &prealloc); } if (flags & SECP256K1_FLAGS_BIT_CONTEXT_VERIFY) { - secp256k1_ecmult_context_build(&ret->ecmult_ctx, &ret->error_callback); + secp256k1_ecmult_context_build(&ret->ecmult_ctx, &prealloc); } - return ret; + return (secp256k1_context*) ret; +} + +secp256k1_context* secp256k1_context_create(unsigned int flags) { + size_t const prealloc_size = secp256k1_context_preallocated_size(flags); + secp256k1_context* ctx = (secp256k1_context*)checked_malloc(&default_error_callback, prealloc_size); + if (EXPECT(secp256k1_context_preallocated_create(ctx, flags) == NULL, 0)) { + free(ctx); + return NULL; + } + + return ctx; } secp256k1_context* secp256k1_context_clone(const secp256k1_context* ctx) { - secp256k1_context* ret = (secp256k1_context*)checked_malloc(&ctx->error_callback, sizeof(secp256k1_context)); - ret->illegal_callback = ctx->illegal_callback; - ret->error_callback = ctx->error_callback; - secp256k1_ecmult_context_clone(&ret->ecmult_ctx, &ctx->ecmult_ctx, &ctx->error_callback); - secp256k1_ecmult_gen_context_clone(&ret->ecmult_gen_ctx, &ctx->ecmult_gen_ctx, &ctx->error_callback); + secp256k1_context* ret; + size_t prealloc_size = ROUND_TO_ALIGN(sizeof(secp256k1_context)); + if (secp256k1_ecmult_gen_context_is_built(&ctx->ecmult_gen_ctx)) { + prealloc_size += SECP256K1_ECMULT_GEN_CONTEXT_PREALLOCATED_SIZE; + } + if (secp256k1_ecmult_context_is_built(&ctx->ecmult_ctx)) { + prealloc_size += SECP256K1_ECMULT_CONTEXT_PREALLOCATED_SIZE; + } + ret = checked_malloc(&ctx->error_callback, prealloc_size); + memcpy(ret, ctx, prealloc_size); + secp256k1_ecmult_gen_context_finalize_memcpy(&ret->ecmult_gen_ctx, &ctx->ecmult_gen_ctx); + secp256k1_ecmult_context_finalize_memcpy(&ret->ecmult_ctx, &ctx->ecmult_ctx); return ret; } -void secp256k1_context_destroy(secp256k1_context* ctx) { +void secp256k1_context_preallocated_destroy(secp256k1_context* ctx) { CHECK(ctx != secp256k1_context_no_precomp); if (ctx != NULL) { secp256k1_ecmult_context_clear(&ctx->ecmult_ctx); secp256k1_ecmult_gen_context_clear(&ctx->ecmult_gen_ctx); + } +} +void secp256k1_context_destroy(secp256k1_context* ctx) { + if (ctx != NULL) { + secp256k1_context_preallocated_destroy(ctx); free(ctx); } } diff --git a/src/tests.c b/src/tests.c index 5566560e88957..8f84a89527359 100644 --- a/src/tests.c +++ b/src/tests.c @@ -229,10 +229,6 @@ void run_context_tests(void) { secp256k1_context_set_illegal_callback(vrfy, NULL, NULL); secp256k1_context_set_illegal_callback(sign, NULL, NULL); - /* This shouldn't leak memory, due to already-set tests. */ - secp256k1_ecmult_gen_context_build(&sign->ecmult_gen_ctx, NULL); - secp256k1_ecmult_context_build(&vrfy->ecmult_ctx, NULL); - /* obtain a working nonce */ do { random_scalar_order_test(&nonce);