From 1bf7c056bad64f3a29652c2cb4f3534ec5c897a9 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Thu, 18 Oct 2018 19:09:51 +0200 Subject: [PATCH 1/9] Prepare for manual memory management in preallocated memory * Determine ALIGNMENT more cleverly and move it to util.h * Implement manual_malloc() helper function --- src/scratch_impl.h | 9 ++------- src/util.h | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/scratch_impl.h b/src/scratch_impl.h index abed713b21d2a..5049084e6db0b 100644 --- a/src/scratch_impl.h +++ b/src/scratch_impl.h @@ -7,14 +7,9 @@ #ifndef _SECP256K1_SCRATCH_IMPL_H_ #define _SECP256K1_SCRATCH_IMPL_H_ +#include "util.h" #include "scratch.h" -/* Using 16 bytes alignment because common architectures never have alignment - * requirements above 8 for any of the types we care about. In addition we - * leave some room because currently we don't care about a few bytes. - * TODO: Determine this at configure time. */ -#define ALIGNMENT 16 - static secp256k1_scratch* secp256k1_scratch_create(const secp256k1_callback* error_callback, size_t max_size) { secp256k1_scratch* ret = (secp256k1_scratch*)checked_malloc(error_callback, sizeof(*ret)); if (ret != NULL) { @@ -71,7 +66,7 @@ static void secp256k1_scratch_deallocate_frame(secp256k1_scratch* scratch) { static void *secp256k1_scratch_alloc(secp256k1_scratch* scratch, size_t size) { void *ret; size_t frame = scratch->frame - 1; - size = ((size + ALIGNMENT - 1) / ALIGNMENT) * ALIGNMENT; + size = ROUND_TO_ALIGN(size); if (scratch->frame == 0 || size + scratch->offset[frame] > scratch->frame_size[frame]) { return NULL; diff --git a/src/util.h b/src/util.h index e1f5b764527d9..9deb61bc59d15 100644 --- a/src/util.h +++ b/src/util.h @@ -84,6 +84,47 @@ static SECP256K1_INLINE void *checked_realloc(const secp256k1_callback* cb, void return ret; } +#if defined(__BIGGEST_ALIGNMENT__) +#define ALIGNMENT __BIGGEST_ALIGNMENT__ +#else +/* Using 16 bytes alignment because common architectures never have alignment + * requirements above 8 for any of the types we care about. In addition we + * leave some room because currently we don't care about a few bytes. */ +#define ALIGNMENT 16 +#endif + +#define ROUND_TO_ALIGN(size) (((size + ALIGNMENT - 1) / ALIGNMENT) * ALIGNMENT) + +/* Assume there is a contiguous memory object with bounds [base, base + max_size) + * of which the memory range [base, *prealloc_ptr) is already allocated for usage, + * where *prealloc_ptr is an aligned pointer. In that setting, this functions + * reserves the subobject [*prealloc_ptr, *prealloc_ptr + alloc_size) of + * alloc_size bytes by increasing *prealloc_ptr accordingly, taking into account + * alignment requirements. + * + * The function returns an aligned pointer to the newly allocated subobject. + * + * This is useful for manual memory management: if we're simply given a block + * [base, base + max_size), the caller can use this function to allocate memory + * in this block and keep track of the current allocation state with *prealloc_ptr. + * + * It is VERIFY_CHECKed that there is enough space left in the memory object and + * *prealloc_ptr is aligned relative to base. + */ +static SECP256K1_INLINE void *manual_alloc(void** prealloc_ptr, size_t alloc_size, void* base, size_t max_size) { + size_t aligned_alloc_size = ROUND_TO_ALIGN(alloc_size); + void* ret; + VERIFY_CHECK(prealloc_ptr != NULL); + VERIFY_CHECK(*prealloc_ptr != NULL); + VERIFY_CHECK(base != NULL); + VERIFY_CHECK((unsigned char*)*prealloc_ptr >= (unsigned char*)base); + VERIFY_CHECK(((unsigned char*)*prealloc_ptr - (unsigned char*)base) % ALIGNMENT == 0); + VERIFY_CHECK((unsigned char*)*prealloc_ptr - (unsigned char*)base + aligned_alloc_size <= max_size); + ret = *prealloc_ptr; + *((unsigned char**)prealloc_ptr) += aligned_alloc_size; + return ret; +} + /* Macro for restrict, when available and not in a VERIFY build. */ #if defined(SECP256K1_BUILD) && defined(VERIFY) # define SECP256K1_RESTRICT From ef020de16fda6b11c95abce6a565da8a2d373b52 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Mon, 22 Oct 2018 16:23:09 +0200 Subject: [PATCH 2/9] Add size constants for preallocated memory --- src/ecmult.h | 1 + src/ecmult_gen.h | 1 + src/ecmult_gen_impl.h | 8 ++++++++ src/ecmult_impl.h | 10 +++++++++- src/secp256k1.c | 18 ++++++++++++++++++ 5 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/ecmult.h b/src/ecmult.h index 3d75a960f4247..6ac78bc3e6ca7 100644 --- a/src/ecmult.h +++ b/src/ecmult.h @@ -20,6 +20,7 @@ typedef struct { #endif } secp256k1_ecmult_context; +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, diff --git a/src/ecmult_gen.h b/src/ecmult_gen.h index 7564b7015f0b7..0e23bc1095a7c 100644 --- a/src/ecmult_gen.h +++ b/src/ecmult_gen.h @@ -28,6 +28,7 @@ typedef struct { secp256k1_gej initial; } secp256k1_ecmult_gen_context; +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, diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index d64505dc00107..1188a21367683 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -7,6 +7,7 @@ #ifndef SECP256K1_ECMULT_GEN_IMPL_H #define SECP256K1_ECMULT_GEN_IMPL_H +#include "util.h" #include "scalar.h" #include "group.h" #include "ecmult_gen.h" @@ -14,6 +15,13 @@ #ifdef USE_ECMULT_STATIC_PRECOMPUTATION #include "ecmult_static_context.h" #endif + +#ifndef USE_ECMULT_STATIC_PRECOMPUTATION + static const size_t SECP256K1_ECMULT_GEN_CONTEXT_PREALLOCATED_SIZE = ROUND_TO_ALIGN(sizeof(*((secp256k1_ecmult_gen_context*) NULL)->prec)); +#else + static const size_t SECP256K1_ECMULT_GEN_CONTEXT_PREALLOCATED_SIZE = 0; +#endif + static void secp256k1_ecmult_gen_context_init(secp256k1_ecmult_gen_context *ctx) { ctx->prec = NULL; } diff --git a/src/ecmult_impl.h b/src/ecmult_impl.h index a93a0df3b416d..70db458406c92 100644 --- a/src/ecmult_impl.h +++ b/src/ecmult_impl.h @@ -10,6 +10,7 @@ #include #include +#include "util.h" #include "group.h" #include "scalar.h" #include "ecmult.h" @@ -310,6 +311,13 @@ static void secp256k1_ecmult_odd_multiples_table_storage_var(const int n, secp25 } \ } while(0) +static const size_t SECP256K1_ECMULT_CONTEXT_PREALLOCATED_SIZE = + ROUND_TO_ALIGN(sizeof((*((secp256k1_ecmult_context*) NULL)->pre_g)[0]) * ECMULT_TABLE_SIZE(WINDOW_G)) +#ifdef USE_ENDOMORPHISM + + ROUND_TO_ALIGN(sizeof((*((secp256k1_ecmult_context*) NULL)->pre_g_128)[0]) * ECMULT_TABLE_SIZE(WINDOW_G)) +#endif + ; + static void secp256k1_ecmult_context_init(secp256k1_ecmult_context *ctx) { ctx->pre_g = NULL; #ifdef USE_ENDOMORPHISM @@ -442,7 +450,7 @@ static int secp256k1_ecmult_wnaf(int *wnaf, int len, const secp256k1_scalar *a, CHECK(carry == 0); while (bit < 256) { CHECK(secp256k1_scalar_get_bits(&s, bit++, 1) == 0); - } + } #endif return last_set_bit + 1; } diff --git a/src/secp256k1.c b/src/secp256k1.c index 73adc6e020de6..fe936c53193d9 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -64,6 +64,24 @@ static const secp256k1_context secp256k1_context_no_precomp_ = { }; const secp256k1_context *secp256k1_context_no_precomp = &secp256k1_context_no_precomp_; +size_t secp256k1_context_preallocated_size(unsigned int flags) { + size_t ret = ROUND_TO_ALIGN(sizeof(secp256k1_context)); + + if (EXPECT((flags & SECP256K1_FLAGS_TYPE_MASK) != SECP256K1_FLAGS_TYPE_CONTEXT, 0)) { + secp256k1_callback_call(&default_illegal_callback, + "Invalid flags"); + return 0; + } + + if (flags & SECP256K1_FLAGS_BIT_CONTEXT_SIGN) { + ret += SECP256K1_ECMULT_GEN_CONTEXT_PREALLOCATED_SIZE; + } + if (flags & SECP256K1_FLAGS_BIT_CONTEXT_VERIFY) { + ret += SECP256K1_ECMULT_CONTEXT_PREALLOCATED_SIZE; + } + return ret; +} + secp256k1_context* secp256k1_context_create(unsigned int flags) { secp256k1_context* ret = (secp256k1_context*)checked_malloc(&default_error_callback, sizeof(secp256k1_context)); ret->illegal_callback = default_illegal_callback; From c4fd5dab451ca3983e50b5d45e0b0ca109486ddc Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Mon, 22 Oct 2018 16:25:26 +0200 Subject: [PATCH 3/9] 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); From 5feadde462e5565458d43f9aa797639d39ce2004 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Thu, 25 Oct 2018 17:14:10 +0200 Subject: [PATCH 4/9] Support cloning a context into preallocated memory --- src/secp256k1.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/secp256k1.c b/src/secp256k1.c index 9055f7b6cf06a..4898cf5e878b4 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -82,6 +82,17 @@ size_t secp256k1_context_preallocated_size(unsigned int flags) { return ret; } +size_t secp256k1_context_preallocated_clone_size(const secp256k1_context* ctx) { + size_t ret = ROUND_TO_ALIGN(sizeof(secp256k1_context)); + if (secp256k1_ecmult_gen_context_is_built(&ctx->ecmult_gen_ctx)) { + ret += SECP256K1_ECMULT_GEN_CONTEXT_PREALLOCATED_SIZE; + } + if (secp256k1_ecmult_context_is_built(&ctx->ecmult_ctx)) { + ret += SECP256K1_ECMULT_CONTEXT_PREALLOCATED_SIZE; + } + return ret; +} + secp256k1_context* secp256k1_context_preallocated_create(void* prealloc, unsigned int flags) { void* const base = prealloc; size_t prealloc_size = secp256k1_context_preallocated_size(flags); @@ -120,22 +131,22 @@ secp256k1_context* secp256k1_context_create(unsigned int flags) { return ctx; } -secp256k1_context* secp256k1_context_clone(const secp256k1_context* ctx) { - 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); +secp256k1_context* secp256k1_context_preallocated_clone(const secp256k1_context* ctx, void* prealloc) { + size_t prealloc_size = secp256k1_context_preallocated_clone_size(ctx); + secp256k1_context* ret = (secp256k1_context*)prealloc; 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; } +secp256k1_context* secp256k1_context_clone(const secp256k1_context* ctx) { + size_t prealloc_size = secp256k1_context_preallocated_clone_size(ctx); + secp256k1_context* ret = (secp256k1_context*)checked_malloc(&ctx->error_callback, prealloc_size); + ret = secp256k1_context_preallocated_clone(ctx, ret); + return ret; +} + void secp256k1_context_preallocated_destroy(secp256k1_context* ctx) { CHECK(ctx != secp256k1_context_no_precomp); if (ctx != NULL) { From ba12dd08daf8a0e1b5eb2b29d633e69a546220ef Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Thu, 25 Oct 2018 18:08:14 +0200 Subject: [PATCH 5/9] Check arguments of _preallocated functions --- src/secp256k1.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/secp256k1.c b/src/secp256k1.c index 4898cf5e878b4..9fdd0c47c7889 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -84,6 +84,7 @@ size_t secp256k1_context_preallocated_size(unsigned int flags) { size_t secp256k1_context_preallocated_clone_size(const secp256k1_context* ctx) { size_t ret = ROUND_TO_ALIGN(sizeof(secp256k1_context)); + VERIFY_CHECK(ctx != NULL); if (secp256k1_ecmult_gen_context_is_built(&ctx->ecmult_gen_ctx)) { ret += SECP256K1_ECMULT_GEN_CONTEXT_PREALLOCATED_SIZE; } @@ -95,9 +96,12 @@ size_t secp256k1_context_preallocated_clone_size(const secp256k1_context* ctx) { 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); + size_t prealloc_size; + secp256k1_context* ret; + VERIFY_CHECK(prealloc != NULL); + prealloc_size = secp256k1_context_preallocated_size(flags); + ret = (secp256k1_context*)manual_alloc(&prealloc, sizeof(secp256k1_context), base, prealloc_size); ret->illegal_callback = default_illegal_callback; ret->error_callback = default_error_callback; @@ -132,8 +136,13 @@ secp256k1_context* secp256k1_context_create(unsigned int flags) { } secp256k1_context* secp256k1_context_preallocated_clone(const secp256k1_context* ctx, void* prealloc) { - size_t prealloc_size = secp256k1_context_preallocated_clone_size(ctx); - secp256k1_context* ret = (secp256k1_context*)prealloc; + size_t prealloc_size; + secp256k1_context* ret; + VERIFY_CHECK(ctx != NULL); + ARG_CHECK(prealloc != NULL); + + prealloc_size = secp256k1_context_preallocated_clone_size(ctx); + ret = (secp256k1_context*)prealloc; 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); @@ -141,8 +150,12 @@ secp256k1_context* secp256k1_context_preallocated_clone(const secp256k1_context* } secp256k1_context* secp256k1_context_clone(const secp256k1_context* ctx) { - size_t prealloc_size = secp256k1_context_preallocated_clone_size(ctx); - secp256k1_context* ret = (secp256k1_context*)checked_malloc(&ctx->error_callback, prealloc_size); + secp256k1_context* ret; + size_t prealloc_size; + + VERIFY_CHECK(ctx != NULL); + prealloc_size = secp256k1_context_preallocated_clone_size(ctx); + ret = (secp256k1_context*)checked_malloc(&ctx->error_callback, prealloc_size); ret = secp256k1_context_preallocated_clone(ctx, ret); return ret; } From 814cc78d711baabff052efa42cf4bbb024af6526 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Thu, 25 Oct 2018 20:32:38 +0200 Subject: [PATCH 6/9] Add tests for contexts in preallocated memory --- src/tests.c | 109 +++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 95 insertions(+), 14 deletions(-) diff --git a/src/tests.c b/src/tests.c index 8f84a89527359..b0d5af7d2e44b 100644 --- a/src/tests.c +++ b/src/tests.c @@ -137,23 +137,47 @@ void random_scalar_order(secp256k1_scalar *num) { } while(1); } -void run_context_tests(void) { +void run_context_tests(int use_prealloc) { secp256k1_pubkey pubkey; secp256k1_pubkey zero_pubkey; secp256k1_ecdsa_signature sig; unsigned char ctmp[32]; int32_t ecount; int32_t ecount2; - secp256k1_context *none = secp256k1_context_create(SECP256K1_CONTEXT_NONE); - secp256k1_context *sign = secp256k1_context_create(SECP256K1_CONTEXT_SIGN); - secp256k1_context *vrfy = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY); - secp256k1_context *both = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY); + secp256k1_context *none; + secp256k1_context *sign; + secp256k1_context *vrfy; + secp256k1_context *both; + void *none_prealloc = NULL; + void *sign_prealloc = NULL; + void *vrfy_prealloc = NULL; + void *both_prealloc = NULL; secp256k1_gej pubj; secp256k1_ge pub; secp256k1_scalar msg, key, nonce; secp256k1_scalar sigr, sigs; + if (use_prealloc) { + none_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_NONE)); + sign_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_SIGN)); + vrfy_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_VERIFY)); + both_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY)); + CHECK(none_prealloc != NULL); + CHECK(sign_prealloc != NULL); + CHECK(vrfy_prealloc != NULL); + CHECK(both_prealloc != NULL); + none = secp256k1_context_preallocated_create(none_prealloc, SECP256K1_CONTEXT_NONE); + sign = secp256k1_context_preallocated_create(sign_prealloc, SECP256K1_CONTEXT_SIGN); + vrfy = secp256k1_context_preallocated_create(vrfy_prealloc, SECP256K1_CONTEXT_VERIFY); + both = secp256k1_context_preallocated_create(both_prealloc, SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY); + } else { + none = secp256k1_context_create(SECP256K1_CONTEXT_NONE); + sign = secp256k1_context_create(SECP256K1_CONTEXT_SIGN); + vrfy = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY); + both = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY); + } + memset(&zero_pubkey, 0, sizeof(zero_pubkey)); ecount = 0; @@ -163,14 +187,57 @@ void run_context_tests(void) { secp256k1_context_set_error_callback(sign, counting_illegal_callback_fn, NULL); CHECK(vrfy->error_callback.fn != sign->error_callback.fn); + /* check if sizes for cloning are consistent */ + CHECK(secp256k1_context_preallocated_clone_size(none) == secp256k1_context_preallocated_size(SECP256K1_CONTEXT_NONE)); + CHECK(secp256k1_context_preallocated_clone_size(sign) == secp256k1_context_preallocated_size(SECP256K1_CONTEXT_SIGN)); + CHECK(secp256k1_context_preallocated_clone_size(vrfy) == secp256k1_context_preallocated_size(SECP256K1_CONTEXT_VERIFY)); + CHECK(secp256k1_context_preallocated_clone_size(both) == secp256k1_context_preallocated_size(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY)); + /*** clone and destroy all of them to make sure cloning was complete ***/ { secp256k1_context *ctx_tmp; - ctx_tmp = none; none = secp256k1_context_clone(none); secp256k1_context_destroy(ctx_tmp); - ctx_tmp = sign; sign = secp256k1_context_clone(sign); secp256k1_context_destroy(ctx_tmp); - ctx_tmp = vrfy; vrfy = secp256k1_context_clone(vrfy); secp256k1_context_destroy(ctx_tmp); - ctx_tmp = both; both = secp256k1_context_clone(both); secp256k1_context_destroy(ctx_tmp); + if (use_prealloc) { + /* clone into a non-preallocated context and then again into a new preallocated one. */ + ctx_tmp = none; none = secp256k1_context_clone(none); secp256k1_context_preallocated_destroy(ctx_tmp); + free(none_prealloc); none_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_NONE)); CHECK(none_prealloc != NULL); + ctx_tmp = none; none = secp256k1_context_preallocated_clone(none, none_prealloc); secp256k1_context_destroy(ctx_tmp); + + ctx_tmp = sign; sign = secp256k1_context_clone(sign); secp256k1_context_preallocated_destroy(ctx_tmp); + free(sign_prealloc); sign_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_SIGN)); CHECK(sign_prealloc != NULL); + ctx_tmp = sign; sign = secp256k1_context_preallocated_clone(sign, sign_prealloc); secp256k1_context_destroy(ctx_tmp); + + ctx_tmp = vrfy; vrfy = secp256k1_context_clone(vrfy); secp256k1_context_preallocated_destroy(ctx_tmp); + free(vrfy_prealloc); vrfy_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_VERIFY)); CHECK(vrfy_prealloc != NULL); + ctx_tmp = vrfy; vrfy = secp256k1_context_preallocated_clone(vrfy, vrfy_prealloc); secp256k1_context_destroy(ctx_tmp); + + ctx_tmp = both; both = secp256k1_context_clone(both); secp256k1_context_preallocated_destroy(ctx_tmp); + free(both_prealloc); both_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY)); CHECK(both_prealloc != NULL); + ctx_tmp = both; both = secp256k1_context_preallocated_clone(both, both_prealloc); secp256k1_context_destroy(ctx_tmp); + } else { + /* clone into a preallocated context and then again into a new non-preallocated one. */ + void *prealloc_tmp; + + prealloc_tmp = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_NONE)); CHECK(prealloc_tmp != NULL); + ctx_tmp = none; none = secp256k1_context_preallocated_clone(none, prealloc_tmp); secp256k1_context_destroy(ctx_tmp); + ctx_tmp = none; none = secp256k1_context_clone(none); secp256k1_context_preallocated_destroy(ctx_tmp); + free(prealloc_tmp); + + prealloc_tmp = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_SIGN)); CHECK(prealloc_tmp != NULL); + ctx_tmp = sign; sign = secp256k1_context_preallocated_clone(sign, prealloc_tmp); secp256k1_context_destroy(ctx_tmp); + ctx_tmp = sign; sign = secp256k1_context_clone(sign); secp256k1_context_preallocated_destroy(ctx_tmp); + free(prealloc_tmp); + + prealloc_tmp = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_VERIFY)); CHECK(prealloc_tmp != NULL); + ctx_tmp = vrfy; vrfy = secp256k1_context_preallocated_clone(vrfy, prealloc_tmp); secp256k1_context_destroy(ctx_tmp); + ctx_tmp = vrfy; vrfy = secp256k1_context_clone(vrfy); secp256k1_context_preallocated_destroy(ctx_tmp); + free(prealloc_tmp); + + prealloc_tmp = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY)); CHECK(prealloc_tmp != NULL); + ctx_tmp = both; both = secp256k1_context_preallocated_clone(both, prealloc_tmp); secp256k1_context_destroy(ctx_tmp); + ctx_tmp = both; both = secp256k1_context_clone(both); secp256k1_context_preallocated_destroy(ctx_tmp); + free(prealloc_tmp); + } } /* Verify that the error callback makes it across the clone. */ @@ -243,12 +310,25 @@ void run_context_tests(void) { CHECK(secp256k1_ecdsa_sig_verify(&both->ecmult_ctx, &sigr, &sigs, &pub, &msg)); /* cleanup */ - secp256k1_context_destroy(none); - secp256k1_context_destroy(sign); - secp256k1_context_destroy(vrfy); - secp256k1_context_destroy(both); + if (use_prealloc) { + secp256k1_context_preallocated_destroy(none); + secp256k1_context_preallocated_destroy(sign); + secp256k1_context_preallocated_destroy(vrfy); + secp256k1_context_preallocated_destroy(both); + free(none_prealloc); + free(sign_prealloc); + free(vrfy_prealloc); + free(both_prealloc); + } else { + secp256k1_context_destroy(none); + secp256k1_context_destroy(sign); + secp256k1_context_destroy(vrfy); + secp256k1_context_destroy(both); + } /* Defined as no-op. */ secp256k1_context_destroy(NULL); + secp256k1_context_preallocated_destroy(NULL); + } void run_scratch_tests(void) { @@ -5058,7 +5138,8 @@ int main(int argc, char **argv) { printf("random seed = %02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x\n", seed16[0], seed16[1], seed16[2], seed16[3], seed16[4], seed16[5], seed16[6], seed16[7], seed16[8], seed16[9], seed16[10], seed16[11], seed16[12], seed16[13], seed16[14], seed16[15]); /* initialize */ - run_context_tests(); + run_context_tests(0); + run_context_tests(1); run_scratch_tests(); ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY); if (secp256k1_rand_bits(1)) { From 695feb6fbde6e410d36dfac60948c9cd87fb976f Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Tue, 27 Nov 2018 16:47:46 +0100 Subject: [PATCH 7/9] Export _preallocated functions --- include/secp256k1.h | 102 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 94 insertions(+), 8 deletions(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index b3b5d0e5591d2..77ca1b95c3168 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -33,9 +33,10 @@ extern "C" { * verification). * * A constructed context can safely be used from multiple threads - * simultaneously, but API call that take a non-const pointer to a context + * simultaneously, but API calls that take a non-const pointer to a context * need exclusive access to it. In particular this is the case for - * secp256k1_context_destroy and secp256k1_context_randomize. + * secp256k1_context_destroy, secp256k1_context_preallocated_destroy, + * and secp256k1_context_randomize. * * Regarding randomization, either do it once at creation time (in which case * you do not need any locking for the other calls), or use a read-write lock. @@ -163,7 +164,8 @@ typedef int (*secp256k1_nonce_function)( #define SECP256K1_FLAGS_BIT_CONTEXT_SIGN (1 << 9) #define SECP256K1_FLAGS_BIT_COMPRESSION (1 << 8) -/** Flags to pass to secp256k1_context_create. */ +/** Flags to pass to secp256k1_context_create, secp256k1_context_preallocated_size, and + * secp256k1_context_preallocated_create. */ #define SECP256K1_CONTEXT_VERIFY (SECP256K1_FLAGS_TYPE_CONTEXT | SECP256K1_FLAGS_BIT_CONTEXT_VERIFY) #define SECP256K1_CONTEXT_SIGN (SECP256K1_FLAGS_TYPE_CONTEXT | SECP256K1_FLAGS_BIT_CONTEXT_SIGN) #define SECP256K1_CONTEXT_NONE (SECP256K1_FLAGS_TYPE_CONTEXT) @@ -186,7 +188,7 @@ typedef int (*secp256k1_nonce_function)( */ SECP256K1_API extern const secp256k1_context *secp256k1_context_no_precomp; -/** Create a secp256k1 context object. +/** Create a secp256k1 context object (in dynamically allocated memory). * * This function uses malloc to allocate memory. It is guaranteed that malloc is * called at most once for every call of this function. @@ -200,7 +202,7 @@ SECP256K1_API secp256k1_context* secp256k1_context_create( unsigned int flags ) SECP256K1_WARN_UNUSED_RESULT; -/** Copies a secp256k1 context object. +/** Copy a secp256k1 context object (into dynamically allocated memory). * * This function uses malloc to allocate memory. It is guaranteed that malloc is * called at most once for every call of this function. @@ -212,14 +214,97 @@ SECP256K1_API secp256k1_context* secp256k1_context_clone( const secp256k1_context* ctx ) SECP256K1_ARG_NONNULL(1) SECP256K1_WARN_UNUSED_RESULT; -/** Destroy a secp256k1 context object. +/** Destroy a secp256k1 context object (created in dynamically allocated memory). * * The context pointer may not be used afterwards. - * Args: ctx: an existing context to destroy (cannot be NULL) + * + * The context to destroy must have been created using secp256k1_context_create + * or secp256k1_context_clone. If the context has instead been created using + * secp256k1_context_preallocated_create or secp256k1_context_preallocated_clone, the + * behaviour is undefined. In that case, secp256k1_context_preallocated_destroy must + * be used instead. + * + * Args: ctx: an existing context to destroy, constructed using + * secp256k1_context_create or secp256k1_context_clone */ SECP256K1_API void secp256k1_context_destroy( secp256k1_context* ctx ); +/** Determine the memory size of a secp256k1 context object to be created in + * caller-provided memory. + * + * The purpose of this function is to determine how much memory must be provided + * to secp256k1_context_preallocated_create. + * + * Returns: the required size of the caller-provided memory block + * In: flags: which parts of the context to initialize. + */ + +SECP256K1_API size_t secp256k1_context_preallocated_size( + unsigned int flags +) SECP256K1_WARN_UNUSED_RESULT; + +/** Create a secp256k1 context object in caller-provided memory. + * + * Returns: a newly created context object. + * In: prealloc: a pointer to a rewritable contiguous block of memory of + * size at least secp256k1_context_preallocated_size(flags) + * bytes, suitably aligned to hold an object of any type + * (cannot be NULL) + * flags: which parts of the context to initialize. + * + * See also secp256k1_context_randomize. + */ +SECP256K1_API secp256k1_context* secp256k1_context_preallocated_create( + void* prealloc, + unsigned int flags +) SECP256K1_ARG_NONNULL(1) SECP256K1_WARN_UNUSED_RESULT; + +/** Determine the memory size of a secp256k1 context object to be copied into + * caller-provided memory. + * + * The purpose of this function is to determine how much memory must be provided + * to secp256k1_context_preallocated_clone when copying the context ctx. + * + * Returns: the required size of the caller-provided memory block. + * In: ctx: an existing context to copy (cannot be NULL) + */ +SECP256K1_API size_t secp256k1_context_preallocated_clone_size( + const secp256k1_context* ctx +) SECP256K1_ARG_NONNULL(1) SECP256K1_WARN_UNUSED_RESULT; + +/** Copy a secp256k1 context object into caller-provided memory. + * + * Returns: a newly created context object. + * Args: ctx: an existing context to copy (cannot be NULL) + * In: prealloc: a pointer to a rewritable contiguous block of memory of + * size at least secp256k1_context_preallocated_size(flags) + * bytes, suitably aligned to hold an object of any type + * (cannot be NULL) + */ +SECP256K1_API secp256k1_context* secp256k1_context_preallocated_clone( + const secp256k1_context* ctx, + void* prealloc +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_WARN_UNUSED_RESULT; + +/** Destroy a secp256k1 context object that has been created in + * caller-provided memory. + * + * The context pointer may not be used afterwards. + * + * The context to destroy must have been created using + * secp256k1_context_preallocated_create or secp256k1_context_preallocated_clone. + * If the context has instead been created using secp256k1_context_create or + * secp256k1_context_clone, the behaviour is undefined. In that case, + * secp256k1_context_destroy must be used instead. + * + * Args: ctx: an existing context to destroy, constructed using + * secp256k1_context_preallocated_create or + * secp256k1_context_preallocated_clone (cannot be NULL) + */ +SECP256K1_API void secp256k1_context_preallocated_destroy( + secp256k1_context* ctx +); /** Set a callback function to be called when an illegal argument is passed to * an API call. It will only trigger for violations that are mentioned @@ -642,7 +727,8 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_mul( * contexts not initialized for signing; then it will have no effect and return 1. * * You should call this after secp256k1_context_create or - * secp256k1_context_clone, and may call this repeatedly afterwards. + * secp256k1_context_clone (and secp256k1_context_preallocated_create or + * secp256k1_context_clone, resp.), and you may call this repeatedly afterwards. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_context_randomize( secp256k1_context* ctx, From 238305fdbb37a68eb9636f707c83fd25719c5195 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Tue, 27 Nov 2018 16:48:57 +0100 Subject: [PATCH 8/9] Move _preallocated functions to separate header --- Makefile.am | 1 + include/secp256k1.h | 81 ++------------------------ include/secp256k1_preallocated.h | 98 ++++++++++++++++++++++++++++++++ src/secp256k1.c | 1 + src/tests.c | 1 + 5 files changed, 105 insertions(+), 77 deletions(-) create mode 100644 include/secp256k1_preallocated.h diff --git a/Makefile.am b/Makefile.am index 9e5b7dcce0ab9..21df09f41fb35 100644 --- a/Makefile.am +++ b/Makefile.am @@ -8,6 +8,7 @@ else JNI_LIB = endif include_HEADERS = include/secp256k1.h +include_HEADERS += include/secp256k1_preallocated.h noinst_HEADERS = noinst_HEADERS += src/scalar.h noinst_HEADERS += src/scalar_4x64.h diff --git a/include/secp256k1.h b/include/secp256k1.h index 77ca1b95c3168..e0d87a1883d2e 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -191,7 +191,8 @@ SECP256K1_API extern const secp256k1_context *secp256k1_context_no_precomp; /** Create a secp256k1 context object (in dynamically allocated memory). * * This function uses malloc to allocate memory. It is guaranteed that malloc is - * called at most once for every call of this function. + * called at most once for every call of this function. If you need to avoid dynamic + * memory allocation entirely, see the functions in secp256k1_preallocated.h. * * Returns: a newly created context object. * In: flags: which parts of the context to initialize. @@ -205,7 +206,8 @@ SECP256K1_API secp256k1_context* secp256k1_context_create( /** Copy a secp256k1 context object (into dynamically allocated memory). * * This function uses malloc to allocate memory. It is guaranteed that malloc is - * called at most once for every call of this function. + * called at most once for every call of this function. If you need to avoid dynamic + * memory allocation entirely, see the functions in secp256k1_preallocated.h. * * Returns: a newly created context object. * Args: ctx: an existing context to copy (cannot be NULL) @@ -230,81 +232,6 @@ SECP256K1_API secp256k1_context* secp256k1_context_clone( SECP256K1_API void secp256k1_context_destroy( secp256k1_context* ctx ); -/** Determine the memory size of a secp256k1 context object to be created in - * caller-provided memory. - * - * The purpose of this function is to determine how much memory must be provided - * to secp256k1_context_preallocated_create. - * - * Returns: the required size of the caller-provided memory block - * In: flags: which parts of the context to initialize. - */ - -SECP256K1_API size_t secp256k1_context_preallocated_size( - unsigned int flags -) SECP256K1_WARN_UNUSED_RESULT; - -/** Create a secp256k1 context object in caller-provided memory. - * - * Returns: a newly created context object. - * In: prealloc: a pointer to a rewritable contiguous block of memory of - * size at least secp256k1_context_preallocated_size(flags) - * bytes, suitably aligned to hold an object of any type - * (cannot be NULL) - * flags: which parts of the context to initialize. - * - * See also secp256k1_context_randomize. - */ -SECP256K1_API secp256k1_context* secp256k1_context_preallocated_create( - void* prealloc, - unsigned int flags -) SECP256K1_ARG_NONNULL(1) SECP256K1_WARN_UNUSED_RESULT; - -/** Determine the memory size of a secp256k1 context object to be copied into - * caller-provided memory. - * - * The purpose of this function is to determine how much memory must be provided - * to secp256k1_context_preallocated_clone when copying the context ctx. - * - * Returns: the required size of the caller-provided memory block. - * In: ctx: an existing context to copy (cannot be NULL) - */ -SECP256K1_API size_t secp256k1_context_preallocated_clone_size( - const secp256k1_context* ctx -) SECP256K1_ARG_NONNULL(1) SECP256K1_WARN_UNUSED_RESULT; - -/** Copy a secp256k1 context object into caller-provided memory. - * - * Returns: a newly created context object. - * Args: ctx: an existing context to copy (cannot be NULL) - * In: prealloc: a pointer to a rewritable contiguous block of memory of - * size at least secp256k1_context_preallocated_size(flags) - * bytes, suitably aligned to hold an object of any type - * (cannot be NULL) - */ -SECP256K1_API secp256k1_context* secp256k1_context_preallocated_clone( - const secp256k1_context* ctx, - void* prealloc -) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_WARN_UNUSED_RESULT; - -/** Destroy a secp256k1 context object that has been created in - * caller-provided memory. - * - * The context pointer may not be used afterwards. - * - * The context to destroy must have been created using - * secp256k1_context_preallocated_create or secp256k1_context_preallocated_clone. - * If the context has instead been created using secp256k1_context_create or - * secp256k1_context_clone, the behaviour is undefined. In that case, - * secp256k1_context_destroy must be used instead. - * - * Args: ctx: an existing context to destroy, constructed using - * secp256k1_context_preallocated_create or - * secp256k1_context_preallocated_clone (cannot be NULL) - */ -SECP256K1_API void secp256k1_context_preallocated_destroy( - secp256k1_context* ctx -); /** Set a callback function to be called when an illegal argument is passed to * an API call. It will only trigger for violations that are mentioned diff --git a/include/secp256k1_preallocated.h b/include/secp256k1_preallocated.h new file mode 100644 index 0000000000000..77426ee786fd5 --- /dev/null +++ b/include/secp256k1_preallocated.h @@ -0,0 +1,98 @@ +#ifndef SECP256K1_PREALLOCATED_H +#define SECP256K1_PREALLOCATED_H + +#include "secp256k1.h" + +#ifdef __cplusplus +extern "C" { +#endif + +/* The module provided by this header file is intended for settings in which it + * is not possible or desirable to rely on dynamic memory allocation. It provides + * functions for creating, cloning, and destroying secp256k1 context objects in a + * contiguous fixed-size block of memory provided by the caller. + * + * It is guaranteed that functions in this module will not call malloc or its + * friends realloc, calloc, and free. + */ + +/** Determine the memory size of a secp256k1 context object to be created in + * caller-provided memory. + * + * The purpose of this function is to determine how much memory must be provided + * to secp256k1_context_preallocated_create. + * + * Returns: the required size of the caller-provided memory block + * In: flags: which parts of the context to initialize. + */ +SECP256K1_API size_t secp256k1_context_preallocated_size( + unsigned int flags +) SECP256K1_WARN_UNUSED_RESULT; + +/** Create a secp256k1 context object in caller-provided memory. + * + * Returns: a newly created context object. + * In: prealloc: a pointer to a rewritable contiguous block of memory of + * size at least secp256k1_context_preallocated_size(flags) + * bytes, suitably aligned to hold an object of any type + * (cannot be NULL) + * flags: which parts of the context to initialize. + * + * See also secp256k1_context_randomize. + */ +SECP256K1_API secp256k1_context* secp256k1_context_preallocated_create( + void* prealloc, + unsigned int flags +) SECP256K1_ARG_NONNULL(1) SECP256K1_WARN_UNUSED_RESULT; + +/** Determine the memory size of a secp256k1 context object to be copied into + * caller-provided memory. + * + * The purpose of this function is to determine how much memory must be provided + * to secp256k1_context_preallocated_clone when copying the context ctx. + * + * Returns: the required size of the caller-provided memory block. + * In: ctx: an existing context to copy (cannot be NULL) + */ +SECP256K1_API size_t secp256k1_context_preallocated_clone_size( + const secp256k1_context* ctx +) SECP256K1_ARG_NONNULL(1) SECP256K1_WARN_UNUSED_RESULT; + +/** Copy a secp256k1 context object into caller-provided memory. + * + * Returns: a newly created context object. + * Args: ctx: an existing context to copy (cannot be NULL) + * In: prealloc: a pointer to a rewritable contiguous block of memory of + * size at least secp256k1_context_preallocated_size(flags) + * bytes, suitably aligned to hold an object of any type + * (cannot be NULL) + */ +SECP256K1_API secp256k1_context* secp256k1_context_preallocated_clone( + const secp256k1_context* ctx, + void* prealloc +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_WARN_UNUSED_RESULT; + +/** Destroy a secp256k1 context object that has been created in + * caller-provided memory. + * + * The context pointer may not be used afterwards. + * + * The context to destroy must have been created using + * secp256k1_context_preallocated_create or secp256k1_context_preallocated_clone. + * If the context has instead been created using secp256k1_context_create or + * secp256k1_context_clone, the behaviour is undefined. In that case, + * secp256k1_context_destroy must be used instead. + * + * Args: ctx: an existing context to destroy, constructed using + * secp256k1_context_preallocated_create or + * secp256k1_context_preallocated_clone (cannot be NULL) + */ +SECP256K1_API void secp256k1_context_preallocated_destroy( + secp256k1_context* ctx +); + +#ifdef __cplusplus +} +#endif + +#endif /* SECP256K1_PREALLOCATED_H */ diff --git a/src/secp256k1.c b/src/secp256k1.c index 9fdd0c47c7889..edfe1f397270a 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -5,6 +5,7 @@ **********************************************************************/ #include "include/secp256k1.h" +#include "include/secp256k1_preallocated.h" #include "util.h" #include "num_impl.h" diff --git a/src/tests.c b/src/tests.c index b0d5af7d2e44b..908858bf388d6 100644 --- a/src/tests.c +++ b/src/tests.c @@ -16,6 +16,7 @@ #include "secp256k1.c" #include "include/secp256k1.h" +#include "include/secp256k1_preallocated.h" #include "testrand_impl.h" #ifdef ENABLE_OPENSSL_TESTS From 0522caac8f28e9eaa19bfebc88e463fc8d409b7c Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Fri, 29 Mar 2019 22:27:01 +0100 Subject: [PATCH 9/9] Explain caller's obligations for preallocated memory --- include/secp256k1_preallocated.h | 48 ++++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/include/secp256k1_preallocated.h b/include/secp256k1_preallocated.h index 77426ee786fd5..0fb64a5431f8a 100644 --- a/include/secp256k1_preallocated.h +++ b/include/secp256k1_preallocated.h @@ -12,7 +12,14 @@ extern "C" { * functions for creating, cloning, and destroying secp256k1 context objects in a * contiguous fixed-size block of memory provided by the caller. * - * It is guaranteed that functions in this module will not call malloc or its + * Context objects created by functions in this module can be used like contexts + * objects created by functions in secp256k1.h, i.e., they can be passed to any + * API function that excepts a context object (see secp256k1.h for details). The + * only exception is that context objects created by functions in this module + * must be destroyed using secp256k1_context_preallocated_destroy (in this + * module) instead of secp256k1_context_destroy (in secp256k1.h). + * + * It is guaranteed that functions in by this module will not call malloc or its * friends realloc, calloc, and free. */ @@ -30,15 +37,29 @@ SECP256K1_API size_t secp256k1_context_preallocated_size( ) SECP256K1_WARN_UNUSED_RESULT; /** Create a secp256k1 context object in caller-provided memory. + * + * The caller must provide a pointer to a rewritable contiguous block of memory + * of size at least secp256k1_context_preallocated_size(flags) bytes, suitably + * aligned to hold an object of any type. + * + * The block of memory is exclusively owned by the created context object during + * the lifetime of this context object, which begins with the call to this + * function and ends when a call to secp256k1_context_preallocated_destroy + * (which destroys the context object again) returns. During the lifetime of the + * context object, the caller is obligated not to access this block of memory, + * i.e., the caller may not read or write the memory, e.g., by copying the memory + * contents to a different location or trying to create a second context object + * in the memory. In simpler words, the prealloc pointer (or any pointer derived + * from it) should not be used during the lifetime of the context object. * * Returns: a newly created context object. * In: prealloc: a pointer to a rewritable contiguous block of memory of * size at least secp256k1_context_preallocated_size(flags) - * bytes, suitably aligned to hold an object of any type - * (cannot be NULL) + * bytes, as detailed above (cannot be NULL) * flags: which parts of the context to initialize. * - * See also secp256k1_context_randomize. + * See also secp256k1_context_randomize (in secp256k1.h) + * and secp256k1_context_preallocated_destroy. */ SECP256K1_API secp256k1_context* secp256k1_context_preallocated_create( void* prealloc, @@ -48,9 +69,6 @@ SECP256K1_API secp256k1_context* secp256k1_context_preallocated_create( /** Determine the memory size of a secp256k1 context object to be copied into * caller-provided memory. * - * The purpose of this function is to determine how much memory must be provided - * to secp256k1_context_preallocated_clone when copying the context ctx. - * * Returns: the required size of the caller-provided memory block. * In: ctx: an existing context to copy (cannot be NULL) */ @@ -59,13 +77,20 @@ SECP256K1_API size_t secp256k1_context_preallocated_clone_size( ) SECP256K1_ARG_NONNULL(1) SECP256K1_WARN_UNUSED_RESULT; /** Copy a secp256k1 context object into caller-provided memory. + * + * The caller must provide a pointer to a rewritable contiguous block of memory + * of size at least secp256k1_context_preallocated_size(flags) bytes, suitably + * aligned to hold an object of any type. + * + * The block of memory is exclusively owned by the created context object during + * the lifetime of this context object, see the description of + * secp256k1_context_preallocated_create for details. * * Returns: a newly created context object. * Args: ctx: an existing context to copy (cannot be NULL) * In: prealloc: a pointer to a rewritable contiguous block of memory of * size at least secp256k1_context_preallocated_size(flags) - * bytes, suitably aligned to hold an object of any type - * (cannot be NULL) + * bytes, as detailed above (cannot be NULL) */ SECP256K1_API secp256k1_context* secp256k1_context_preallocated_clone( const secp256k1_context* ctx, @@ -83,6 +108,11 @@ SECP256K1_API secp256k1_context* secp256k1_context_preallocated_clone( * secp256k1_context_clone, the behaviour is undefined. In that case, * secp256k1_context_destroy must be used instead. * + * If required, it is the responsibility of the caller to deallocate the block + * of memory properly after this function returns, e.g., by calling free on the + * preallocated pointer given to secp256k1_context_preallocated_create or + * secp256k1_context_preallocated_clone. + * * Args: ctx: an existing context to destroy, constructed using * secp256k1_context_preallocated_create or * secp256k1_context_preallocated_clone (cannot be NULL)