Skip to content

Commit d3454ad

Browse files
sipasipa
committed
Abstract interactions with valgrind behind new checkmem.h
1 parent dcb0cc0 commit d3454ad

11 files changed

+199
-152
lines changed

‎Makefile.am

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ noinst_HEADERS += src/modinv64_impl.h
4747
noinst_HEADERS += src/precomputed_ecmult.h
4848
noinst_HEADERS += src/precomputed_ecmult_gen.h
4949
noinst_HEADERS += src/assumptions.h
50+
noinst_HEADERS += src/checkmem.h
5051
noinst_HEADERS += src/util.h
5152
noinst_HEADERS += src/int128.h
5253
noinst_HEADERS += src/int128_impl.h

‎src/checkmem.h

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/***********************************************************************
2+
* Copyright (c) 2022 Pieter Wuille *
3+
* Distributed under the MIT software license, see the accompanying *
4+
* file COPYING or https://www.opensource.org/licenses/mit-license.php.*
5+
***********************************************************************/
6+
7+
/* The code here is inspired by Kris Kwiatkowski's approach in
8+
* https://github.com/kriskwiatkowski/pqc/blob/main/src/common/ct_check.h
9+
* to provide a general interface for memory-checking mechanisms, primarily
10+
* for constant-time checking.
11+
*/
12+
13+
/* These macros are defined by this header file:
14+
*
15+
* - SECP256K1_CHECKMEM_ENABLED:
16+
* - 1 if memory-checking integration is available, 0 otherwise.
17+
* This is just a compile-time macro. Use the next macro to check it is actually
18+
* available at runtime.
19+
* - SECP256K1_CHECKMEM_RUNNING():
20+
* - Acts like a function call, returning 1 if memory checking is available
21+
* at runtime.
22+
* - SECP256K1_CHECKMEM_CHECK(p, len):
23+
* - Assert or otherwise fail in case the len-byte memory block pointed to by p is
24+
* not considered entirely defined.
25+
* - SECP256K1_CHECKMEM_CHECK_VERIFY(p, len):
26+
* - Like SECP256K1_CHECKMEM_CHECK, but only works in VERIFY mode.
27+
* - SECP256K1_CHECKMEM_UNDEFINE(p, len):
28+
* - marks the len-byte memory block pointed to by p as undefined data (secret data,
29+
* in the context of constant-time checking).
30+
* - SECP256K1_CHECKMEM_DEFINE(p, len):
31+
* - marks the len-byte memory pointed to by p as defined data (public data, in the
32+
* context of constant-time checking).
33+
*
34+
*/
35+
36+
#ifndef SECP256K1_CHECKMEM_H
37+
#define SECP256K1_CHECKMEM_H
38+
39+
/* Define a statement-like macro that ignores the arguments. */
40+
#define SECP256K1_CHECKMEM_NOOP(p, len) do { (void)(p); (void)(len); } while(0)
41+
42+
/* If valgrind integration is desired (through the VALGRIND define), implement the
43+
* SECP256K1_CHECKMEM_* macros using valgrind. */
44+
#if !defined SECP256K1_CHECKMEM_ENABLED
45+
# if defined VALGRIND
46+
# include <valgrind/memcheck.h>
47+
# define SECP256K1_CHECKMEM_ENABLED 1
48+
# define SECP256K1_CHECKMEM_UNDEFINE(p, len) VALGRIND_MAKE_MEM_UNDEFINED((p), (len))
49+
# define SECP256K1_CHECKMEM_DEFINE(p, len) VALGRIND_MAKE_MEM_DEFINED((p), (len))
50+
# define SECP256K1_CHECKMEM_CHECK(p, len) VALGRIND_CHECK_MEM_IS_DEFINED((p), (len))
51+
# define SECP256K1_CHECKMEM_RUNNING() (RUNNING_ON_VALGRIND)
52+
# endif
53+
#endif
54+
55+
/* As a fall-back, map these macros to dummy statements. */
56+
#if !defined SECP256K1_CHECKMEM_ENABLED
57+
# define SECP256K1_CHECKMEM_ENABLED 0
58+
# define SECP256K1_CHECKMEM_UNDEFINE(p, len) SECP256K1_CHECKMEM_NOOP((p), (len))
59+
# define SECP256K1_CHECKMEM_DEFINE(p, len) SECP256K1_CHECKMEM_NOOP((p), (len))
60+
# define SECP256K1_CHECKMEM_CHECK(p, len) SECP256K1_CHECKMEM_NOOP((p), (len))
61+
# define SECP256K1_CHECKMEM_RUNNING() (0)
62+
#endif
63+
64+
#if defined VERIFY
65+
#define SECP256K1_CHECKMEM_CHECK_VERIFY(p, len) SECP256K1_CHECKMEM_CHECK((p), (len))
66+
#else
67+
#define SECP256K1_CHECKMEM_CHECK_VERIFY(p, len) SECP256K1_CHECKMEM_NOOP((p), (len))
68+
#endif
69+
70+
#endif /* SECP256K1_CHECKMEM_H */

‎src/field_10x26_impl.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#ifndef SECP256K1_FIELD_REPR_IMPL_H
88
#define SECP256K1_FIELD_REPR_IMPL_H
99

10+
#include "checkmem.h"
1011
#include "util.h"
1112
#include "field.h"
1213
#include "modinv32_impl.h"
@@ -1132,7 +1133,7 @@ static void secp256k1_fe_sqr(secp256k1_fe *r, const secp256k1_fe *a) {
11321133

11331134
static SECP256K1_INLINE void secp256k1_fe_cmov(secp256k1_fe *r, const secp256k1_fe *a, int flag) {
11341135
uint32_t mask0, mask1;
1135-
VG_CHECK_VERIFY(r->n, sizeof(r->n));
1136+
SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
11361137
mask0 = flag + ~((uint32_t)0);
11371138
mask1 = ~mask0;
11381139
r->n[0] = (r->n[0] & mask0) | (a->n[0] & mask1);
@@ -1231,7 +1232,7 @@ static SECP256K1_INLINE void secp256k1_fe_half(secp256k1_fe *r) {
12311232

12321233
static SECP256K1_INLINE void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, const secp256k1_fe_storage *a, int flag) {
12331234
uint32_t mask0, mask1;
1234-
VG_CHECK_VERIFY(r->n, sizeof(r->n));
1235+
SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
12351236
mask0 = flag + ~((uint32_t)0);
12361237
mask1 = ~mask0;
12371238
r->n[0] = (r->n[0] & mask0) | (a->n[0] & mask1);

‎src/field_5x52_impl.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#ifndef SECP256K1_FIELD_REPR_IMPL_H
88
#define SECP256K1_FIELD_REPR_IMPL_H
99

10+
#include "checkmem.h"
1011
#include "util.h"
1112
#include "field.h"
1213
#include "modinv64_impl.h"
@@ -472,7 +473,7 @@ static void secp256k1_fe_sqr(secp256k1_fe *r, const secp256k1_fe *a) {
472473

473474
static SECP256K1_INLINE void secp256k1_fe_cmov(secp256k1_fe *r, const secp256k1_fe *a, int flag) {
474475
uint64_t mask0, mask1;
475-
VG_CHECK_VERIFY(r->n, sizeof(r->n));
476+
SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
476477
mask0 = flag + ~((uint64_t)0);
477478
mask1 = ~mask0;
478479
r->n[0] = (r->n[0] & mask0) | (a->n[0] & mask1);
@@ -555,7 +556,7 @@ static SECP256K1_INLINE void secp256k1_fe_half(secp256k1_fe *r) {
555556

556557
static SECP256K1_INLINE void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, const secp256k1_fe_storage *a, int flag) {
557558
uint64_t mask0, mask1;
558-
VG_CHECK_VERIFY(r->n, sizeof(r->n));
559+
SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
559560
mask0 = flag + ~((uint64_t)0);
560561
mask1 = ~mask0;
561562
r->n[0] = (r->n[0] & mask0) | (a->n[0] & mask1);

‎src/scalar_4x64_impl.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#ifndef SECP256K1_SCALAR_REPR_IMPL_H
88
#define SECP256K1_SCALAR_REPR_IMPL_H
99

10+
#include "checkmem.h"
1011
#include "int128.h"
1112
#include "modinv64_impl.h"
1213

@@ -810,7 +811,7 @@ SECP256K1_INLINE static void secp256k1_scalar_mul_shift_var(secp256k1_scalar *r,
810811

811812
static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) {
812813
uint64_t mask0, mask1;
813-
VG_CHECK_VERIFY(r->d, sizeof(r->d));
814+
SECP256K1_CHECKMEM_CHECK_VERIFY(r->d, sizeof(r->d));
814815
mask0 = flag + ~((uint64_t)0);
815816
mask1 = ~mask0;
816817
r->d[0] = (r->d[0] & mask0) | (a->d[0] & mask1);

‎src/scalar_8x32_impl.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#ifndef SECP256K1_SCALAR_REPR_IMPL_H
88
#define SECP256K1_SCALAR_REPR_IMPL_H
99

10+
#include "checkmem.h"
1011
#include "modinv32_impl.h"
1112

1213
/* Limbs of the secp256k1 order. */
@@ -631,7 +632,7 @@ SECP256K1_INLINE static void secp256k1_scalar_mul_shift_var(secp256k1_scalar *r,
631632

632633
static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) {
633634
uint32_t mask0, mask1;
634-
VG_CHECK_VERIFY(r->d, sizeof(r->d));
635+
SECP256K1_CHECKMEM_CHECK_VERIFY(r->d, sizeof(r->d));
635636
mask0 = flag + ~((uint32_t)0);
636637
mask1 = ~mask0;
637638
r->d[0] = (r->d[0] & mask0) | (a->d[0] & mask1);

‎src/scalar_low_impl.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#ifndef SECP256K1_SCALAR_REPR_IMPL_H
88
#define SECP256K1_SCALAR_REPR_IMPL_H
99

10+
#include "checkmem.h"
1011
#include "scalar.h"
1112

1213
#include <string.h>
@@ -115,7 +116,7 @@ SECP256K1_INLINE static int secp256k1_scalar_eq(const secp256k1_scalar *a, const
115116

116117
static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) {
117118
uint32_t mask0, mask1;
118-
VG_CHECK_VERIFY(r, sizeof(*r));
119+
SECP256K1_CHECKMEM_CHECK_VERIFY(r, sizeof(*r));
119120
mask0 = flag + ~((uint32_t)0);
120121
mask1 = ~mask0;
121122
*r = (*r & mask0) | (*a & mask1);

‎src/secp256k1.c

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "../include/secp256k1_preallocated.h"
2222

2323
#include "assumptions.h"
24+
#include "checkmem.h"
2425
#include "util.h"
2526

2627
#include "field_impl.h"
@@ -40,10 +41,6 @@
4041
# error "secp256k1.h processed without SECP256K1_BUILD defined while building secp256k1.c"
4142
#endif
4243

43-
#if defined(VALGRIND)
44-
# include <valgrind/memcheck.h>
45-
#endif
46-
4744
#define ARG_CHECK(cond) do { \
4845
if (EXPECT(!(cond), 0)) { \
4946
secp256k1_callback_call(&ctx->illegal_callback, #cond); \
@@ -199,17 +196,10 @@ void secp256k1_scratch_space_destroy(const secp256k1_context *ctx, secp256k1_scr
199196
}
200197

201198
/* Mark memory as no-longer-secret for the purpose of analysing constant-time behaviour
202-
* of the software. This is setup for use with valgrind but could be substituted with
203-
* the appropriate instrumentation for other analysis tools.
199+
* of the software.
204200
*/
205201
static SECP256K1_INLINE void secp256k1_declassify(const secp256k1_context* ctx, const void *p, size_t len) {
206-
#if defined(VALGRIND)
207-
if (EXPECT(ctx->declassify,0)) VALGRIND_MAKE_MEM_DEFINED(p, len);
208-
#else
209-
(void)ctx;
210-
(void)p;
211-
(void)len;
212-
#endif
202+
if (EXPECT(ctx->declassify, 0)) SECP256K1_CHECKMEM_DEFINE(p, len);
213203
}
214204

215205
static int secp256k1_pubkey_load(const secp256k1_context* ctx, secp256k1_ge* ge, const secp256k1_pubkey* pubkey) {

0 commit comments

Comments
 (0)