Skip to content

Commit

Permalink
Merge #1496: msan: notate variable assignments from assembly code
Browse files Browse the repository at this point in the history
31ba404 msan: notate variable assignments from assembly code (Cory Fields)
e7ea32e msan: Add SECP256K1_CHECKMEM_MSAN_DEFINE which applies to memory sanitizer and not valgrind (Cory Fields)

Pull request description:

  msan isn't smart enough to see that these are set without some help.

  This was pointed out here: #1169 (comment)

  With this commit, msan output is clean even with x86 asm turned on.

ACKs for top commit:
  real-or-random:
    utACK 31ba404
  hebasto:
    re-ACK 31ba404.

Tree-SHA512: c9c51fe542247e1e0a93f6d0063d119cf777ca8c1b7e9c8e45e168a2020dc503872eb2a78004725de81267a3ce78c923be1f8546fb92a3e95fc7ef034e5ba932
  • Loading branch information
real-or-random committed Feb 27, 2024
2 parents 0653a25 + 31ba404 commit d926510
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 0 deletions.
7 changes: 7 additions & 0 deletions src/checkmem.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
* - 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).
* - SECP256K1_CHECKMEM_MSAN_DEFINE(p, len):
* - Like SECP256K1_CHECKMEM_DEFINE, but applies only to memory_sanitizer.
*
*/

Expand All @@ -48,11 +50,16 @@
# define SECP256K1_CHECKMEM_ENABLED 1
# define SECP256K1_CHECKMEM_UNDEFINE(p, len) __msan_allocated_memory((p), (len))
# define SECP256K1_CHECKMEM_DEFINE(p, len) __msan_unpoison((p), (len))
# define SECP256K1_CHECKMEM_MSAN_DEFINE(p, len) __msan_unpoison((p), (len))
# define SECP256K1_CHECKMEM_CHECK(p, len) __msan_check_mem_is_initialized((p), (len))
# define SECP256K1_CHECKMEM_RUNNING() (1)
# endif
#endif

#if !defined SECP256K1_CHECKMEM_MSAN_DEFINE
# define SECP256K1_CHECKMEM_MSAN_DEFINE(p, len) SECP256K1_CHECKMEM_NOOP((p), (len))
#endif

/* If valgrind integration is desired (through the VALGRIND define), implement the
* SECP256K1_CHECKMEM_* macros using valgrind. */
#if !defined SECP256K1_CHECKMEM_ENABLED
Expand Down
18 changes: 18 additions & 0 deletions src/scalar_4x64_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,14 @@ static void secp256k1_scalar_reduce_512(secp256k1_scalar *r, const uint64_t *l)
: "S"(l), "i"(SECP256K1_N_C_0), "i"(SECP256K1_N_C_1)
: "rax", "rdx", "r8", "r9", "r10", "r11", "r12", "r13", "r14", "cc");

SECP256K1_CHECKMEM_MSAN_DEFINE(&m0, sizeof(m0));
SECP256K1_CHECKMEM_MSAN_DEFINE(&m1, sizeof(m1));
SECP256K1_CHECKMEM_MSAN_DEFINE(&m2, sizeof(m2));
SECP256K1_CHECKMEM_MSAN_DEFINE(&m3, sizeof(m3));
SECP256K1_CHECKMEM_MSAN_DEFINE(&m4, sizeof(m4));
SECP256K1_CHECKMEM_MSAN_DEFINE(&m5, sizeof(m5));
SECP256K1_CHECKMEM_MSAN_DEFINE(&m6, sizeof(m6));

/* Reduce 385 bits into 258. */
__asm__ __volatile__(
/* Preload */
Expand Down Expand Up @@ -541,6 +549,12 @@ static void secp256k1_scalar_reduce_512(secp256k1_scalar *r, const uint64_t *l)
: "g"(m0), "g"(m1), "g"(m2), "g"(m3), "g"(m4), "g"(m5), "g"(m6), "i"(SECP256K1_N_C_0), "i"(SECP256K1_N_C_1)
: "rax", "rdx", "r8", "r9", "r10", "r11", "r12", "r13", "cc");

SECP256K1_CHECKMEM_MSAN_DEFINE(&p0, sizeof(p0));
SECP256K1_CHECKMEM_MSAN_DEFINE(&p1, sizeof(p1));
SECP256K1_CHECKMEM_MSAN_DEFINE(&p2, sizeof(p2));
SECP256K1_CHECKMEM_MSAN_DEFINE(&p3, sizeof(p3));
SECP256K1_CHECKMEM_MSAN_DEFINE(&p4, sizeof(p4));

/* Reduce 258 bits into 256. */
__asm__ __volatile__(
/* Preload */
Expand Down Expand Up @@ -586,6 +600,10 @@ static void secp256k1_scalar_reduce_512(secp256k1_scalar *r, const uint64_t *l)
: "=g"(c)
: "g"(p0), "g"(p1), "g"(p2), "g"(p3), "g"(p4), "D"(r), "i"(SECP256K1_N_C_0), "i"(SECP256K1_N_C_1)
: "rax", "rdx", "r8", "r9", "r10", "cc", "memory");

SECP256K1_CHECKMEM_MSAN_DEFINE(r, sizeof(*r));
SECP256K1_CHECKMEM_MSAN_DEFINE(&c, sizeof(c));

#else
secp256k1_uint128 c128;
uint64_t c, c0, c1, c2;
Expand Down

0 comments on commit d926510

Please sign in to comment.