Abstract interactions with valgrind behind new checkmem.h

Makefile.am

@@ -47,6 +47,7 @@ noinst_HEADERS += src/modinv64_impl.h
 noinst_HEADERS += src/precomputed_ecmult.h
 noinst_HEADERS += src/precomputed_ecmult_gen.h
 noinst_HEADERS += src/assumptions.h
+noinst_HEADERS += src/checkmem.h
 noinst_HEADERS += src/util.h
 noinst_HEADERS += src/int128.h
 noinst_HEADERS += src/int128_impl.h

src/checkmem.h

@@ -0,0 +1,74 @@
+/***********************************************************************
+ * Copyright (c) 2022 Pieter Wuille                                    *
+ * Distributed under the MIT software license, see the accompanying    *
+ * file COPYING or https://www.opensource.org/licenses/mit-license.php.*
+ ***********************************************************************/
+
+/* The code here is inspired by Kris Kwiatkowski's approach in
+ * https://github.com/kriskwiatkowski/pqc/blob/main/src/common/ct_check.h
+ * to provide a general interface for memory-checking mechanisms, primarily
+ * for constant-time checking.
+ */
+
+/* These macros are defined by this header file:
+ *
+ * - SECP256K1_CHECKMEM_ENABLED:
+ *   - 1 if memory-checking integration is available, 0 otherwise.
+ *     This is just a compile-time macro. Use the next macro to check it is actually
+ *     available at runtime.
+ * - SECP256K1_CHECKMEM_RUNNING():
+ *   - Acts like a function call, returning 1 if memory checking is available
+ *     at runtime.
+ * - SECP256K1_CHECKMEM_CHECK(p, len):
+ *   - Assert or otherwise fail in case the len-byte memory block pointed to by p is
+ *     not considered entirely defined.
+ * - SECP256K1_CHECKMEM_CHECK_VERIFY(p, len):
+ *   - Like SECP256K1_CHECKMEM_CHECK, but only works in VERIFY mode.
+ * - SECP256K1_CHECKMEM_UNDEFINE(p, len):
+ *   - marks the len-byte memory block pointed to by p as undefined data (secret data,
+ *     in the context of constant-time checking).
+ * - SECP256K1_CHECKMEM_DEFINE(p, len):
+ *   - marks the len-byte memory pointed to by p as defined data (public data, in the
+ *     context of constant-time checking).
+ *
+ */
+
+#ifndef SECP256K1_CHECKMEM_H
+#define SECP256K1_CHECKMEM_H
+
+/* Define a statement-like macro that ignores the arguments. */
+#define SECP256K1_CHECKMEM_NOOP(p, len) do { (void)(p); (void)(len); } while(0)
+
+/* If valgrind integration is desired (through the VALGRIND define), implement the
+ * SECP256K1_CHECKMEM_* macros using valgrind. */
+#if !defined SECP256K1_CHECKMEM_ENABLED
+#  if defined VALGRIND
+#    include <stddef.h>
+#    include <valgrind/memcheck.h>
+#    define SECP256K1_CHECKMEM_ENABLED 1
+#    define SECP256K1_CHECKMEM_UNDEFINE(p, len) VALGRIND_MAKE_MEM_UNDEFINED((p), (len))
+#    define SECP256K1_CHECKMEM_DEFINE(p, len) VALGRIND_MAKE_MEM_DEFINED((p), (len))
+#    define SECP256K1_CHECKMEM_CHECK(p, len) VALGRIND_CHECK_MEM_IS_DEFINED((p), (len))
+     /* VALGRIND_MAKE_MEM_DEFINED returns 0 iff not running on memcheck.
+      * This is more precise than the RUNNING_ON_VALGRIND macro, which
+      * checks for valgrind in general instead of memcheck specifically. */
+#    define SECP256K1_CHECKMEM_RUNNING() (VALGRIND_MAKE_MEM_DEFINED(NULL, 0) != 0)
+#  endif
+#endif
+
+/* As a fall-back, map these macros to dummy statements. */
+#if !defined SECP256K1_CHECKMEM_ENABLED
+#  define SECP256K1_CHECKMEM_ENABLED 0
+#  define SECP256K1_CHECKMEM_UNDEFINE(p, len) SECP256K1_CHECKMEM_NOOP((p), (len))
+#  define SECP256K1_CHECKMEM_DEFINE(p, len) SECP256K1_CHECKMEM_NOOP((p), (len))
+#  define SECP256K1_CHECKMEM_CHECK(p, len) SECP256K1_CHECKMEM_NOOP((p), (len))
+#  define SECP256K1_CHECKMEM_RUNNING() (0)
+#endif
+
+#if defined VERIFY
+#define SECP256K1_CHECKMEM_CHECK_VERIFY(p, len) SECP256K1_CHECKMEM_CHECK((p), (len))
+#else
+#define SECP256K1_CHECKMEM_CHECK_VERIFY(p, len) SECP256K1_CHECKMEM_NOOP((p), (len))
+#endif
+
+#endif /* SECP256K1_CHECKMEM_H */

src/field_10x26_impl.h

@@ -7,6 +7,7 @@
 #ifndef SECP256K1_FIELD_REPR_IMPL_H
 #define SECP256K1_FIELD_REPR_IMPL_H
 
+#include "checkmem.h"
 #include "util.h"
 #include "field.h"
 #include "modinv32_impl.h"
@@ -1132,7 +1133,7 @@ static void secp256k1_fe_sqr(secp256k1_fe *r, const secp256k1_fe *a) {
 
 static SECP256K1_INLINE void secp256k1_fe_cmov(secp256k1_fe *r, const secp256k1_fe *a, int flag) {
     uint32_t mask0, mask1;
-    VG_CHECK_VERIFY(r->n, sizeof(r->n));
+    SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
     mask0 = flag + ~((uint32_t)0);
     mask1 = ~mask0;
     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) {
 
 static SECP256K1_INLINE void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, const secp256k1_fe_storage *a, int flag) {
     uint32_t mask0, mask1;
-    VG_CHECK_VERIFY(r->n, sizeof(r->n));
+    SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
     mask0 = flag + ~((uint32_t)0);
     mask1 = ~mask0;
     r->n[0] = (r->n[0] & mask0) | (a->n[0] & mask1);

src/field_5x52_impl.h

@@ -7,6 +7,7 @@
 #ifndef SECP256K1_FIELD_REPR_IMPL_H
 #define SECP256K1_FIELD_REPR_IMPL_H
 
+#include "checkmem.h"
 #include "util.h"
 #include "field.h"
 #include "modinv64_impl.h"
@@ -472,7 +473,7 @@ static void secp256k1_fe_sqr(secp256k1_fe *r, const secp256k1_fe *a) {
 
 static SECP256K1_INLINE void secp256k1_fe_cmov(secp256k1_fe *r, const secp256k1_fe *a, int flag) {
     uint64_t mask0, mask1;
-    VG_CHECK_VERIFY(r->n, sizeof(r->n));
+    SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
     mask0 = flag + ~((uint64_t)0);
     mask1 = ~mask0;
     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) {
 
 static SECP256K1_INLINE void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, const secp256k1_fe_storage *a, int flag) {
     uint64_t mask0, mask1;
-    VG_CHECK_VERIFY(r->n, sizeof(r->n));
+    SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
     mask0 = flag + ~((uint64_t)0);
     mask1 = ~mask0;
     r->n[0] = (r->n[0] & mask0) | (a->n[0] & mask1);

src/scalar_4x64_impl.h

@@ -7,6 +7,7 @@
 #ifndef SECP256K1_SCALAR_REPR_IMPL_H
 #define SECP256K1_SCALAR_REPR_IMPL_H
 
+#include "checkmem.h"
 #include "int128.h"
 #include "modinv64_impl.h"
 
@@ -810,7 +811,7 @@ SECP256K1_INLINE static void secp256k1_scalar_mul_shift_var(secp256k1_scalar *r,
 
 static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) {
     uint64_t mask0, mask1;
-    VG_CHECK_VERIFY(r->d, sizeof(r->d));
+    SECP256K1_CHECKMEM_CHECK_VERIFY(r->d, sizeof(r->d));
     mask0 = flag + ~((uint64_t)0);
     mask1 = ~mask0;
     r->d[0] = (r->d[0] & mask0) | (a->d[0] & mask1);

src/scalar_8x32_impl.h

@@ -7,6 +7,7 @@
 #ifndef SECP256K1_SCALAR_REPR_IMPL_H
 #define SECP256K1_SCALAR_REPR_IMPL_H
 
+#include "checkmem.h"
 #include "modinv32_impl.h"
 
 /* Limbs of the secp256k1 order. */
@@ -631,7 +632,7 @@ SECP256K1_INLINE static void secp256k1_scalar_mul_shift_var(secp256k1_scalar *r,
 
 static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) {
     uint32_t mask0, mask1;
-    VG_CHECK_VERIFY(r->d, sizeof(r->d));
+    SECP256K1_CHECKMEM_CHECK_VERIFY(r->d, sizeof(r->d));
     mask0 = flag + ~((uint32_t)0);
     mask1 = ~mask0;
     r->d[0] = (r->d[0] & mask0) | (a->d[0] & mask1);

src/scalar_low_impl.h

@@ -7,6 +7,7 @@
 #ifndef SECP256K1_SCALAR_REPR_IMPL_H
 #define SECP256K1_SCALAR_REPR_IMPL_H
 
+#include "checkmem.h"
 #include "scalar.h"
 
 #include <string.h>
@@ -115,7 +116,7 @@ SECP256K1_INLINE static int secp256k1_scalar_eq(const secp256k1_scalar *a, const
 
 static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) {
     uint32_t mask0, mask1;
-    VG_CHECK_VERIFY(r, sizeof(*r));
+    SECP256K1_CHECKMEM_CHECK_VERIFY(r, sizeof(*r));
     mask0 = flag + ~((uint32_t)0);
     mask1 = ~mask0;
     *r = (*r & mask0) | (*a & mask1);

src/secp256k1.c

@@ -21,6 +21,7 @@
 #include "../include/secp256k1_preallocated.h"
 
 #include "assumptions.h"
+#include "checkmem.h"
 #include "util.h"
 
 #include "field_impl.h"
@@ -40,10 +41,6 @@
 # error "secp256k1.h processed without SECP256K1_BUILD defined while building secp256k1.c"
 #endif
 
-#if defined(VALGRIND)
-# include <valgrind/memcheck.h>
-#endif
-
 #define ARG_CHECK(cond) do { \
     if (EXPECT(!(cond), 0)) { \
         secp256k1_callback_call(&ctx->illegal_callback, #cond); \
@@ -215,17 +212,10 @@ void secp256k1_scratch_space_destroy(const secp256k1_context *ctx, secp256k1_scr
 }
 
 /* Mark memory as no-longer-secret for the purpose of analysing constant-time behaviour
- *  of the software. This is setup for use with valgrind but could be substituted with
- *  the appropriate instrumentation for other analysis tools.
+ *  of the software.
  */
 static SECP256K1_INLINE void secp256k1_declassify(const secp256k1_context* ctx, const void *p, size_t len) {
-#if defined(VALGRIND)
-    if (EXPECT(ctx->declassify,0)) VALGRIND_MAKE_MEM_DEFINED(p, len);
-#else
-    (void)ctx;
-    (void)p;
-    (void)len;
-#endif
+    if (EXPECT(ctx->declassify, 0)) SECP256K1_CHECKMEM_DEFINE(p, len);
 }
 
 static int secp256k1_pubkey_load(const secp256k1_context* ctx, secp256k1_ge* ge, const secp256k1_pubkey* pubkey) {