Skip to content

Commit

Permalink
Merge bitcoin-core/secp256k1#1257: ct: Use volatile "trick" in all fe…
Browse files Browse the repository at this point in the history
…/scalar cmov implementations

4a496a3 ct: Use volatile "trick" in all fe/scalar cmov implementations (Tim Ruffing)

Pull request description:

  Apparently clang 15 is able to compile our cmov code into a branch, at least for fe_cmov and fe_storage_cmov. This commit makes the condition volatile in all cmov implementations (except ge but that one only calls into the fe impls).

  This is just a quick fix. We should still look into other methods, e.g., asm and bitcoin#457. We should also consider not caring about constant-time in scalar_low_impl.h

  We should also consider testing on very new compilers in nightly CI, see bitcoin-core/secp256k1#864 (comment)

ACKs for top commit:
  jonasnick:
    ACK 4a496a3

Tree-SHA512: a6010f9d752e45f01f88b804a9b27e77caf5ddf133ddcbc4235b94698bda41c9276bf588c93710e538250d1a96844bcec198ec5459e675f166ceaaa42da921d5
  • Loading branch information
real-or-random committed Apr 6, 2023
2 parents 2bca0a5 + 4a496a3 commit 2d51a45
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 7 deletions.
6 changes: 4 additions & 2 deletions src/field_10x26_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1147,8 +1147,9 @@ 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;
volatile int vflag = flag;
SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
mask0 = flag + ~((uint32_t)0);
mask0 = vflag + ~((uint32_t)0);
mask1 = ~mask0;
r->n[0] = (r->n[0] & mask0) | (a->n[0] & mask1);
r->n[1] = (r->n[1] & mask0) | (a->n[1] & mask1);
Expand Down Expand Up @@ -1246,8 +1247,9 @@ 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;
volatile int vflag = flag;
SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
mask0 = flag + ~((uint32_t)0);
mask0 = vflag + ~((uint32_t)0);
mask1 = ~mask0;
r->n[0] = (r->n[0] & mask0) | (a->n[0] & mask1);
r->n[1] = (r->n[1] & mask0) | (a->n[1] & mask1);
Expand Down
6 changes: 4 additions & 2 deletions src/field_5x52_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -487,8 +487,9 @@ 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;
volatile int vflag = flag;
SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
mask0 = flag + ~((uint64_t)0);
mask0 = vflag + ~((uint64_t)0);
mask1 = ~mask0;
r->n[0] = (r->n[0] & mask0) | (a->n[0] & mask1);
r->n[1] = (r->n[1] & mask0) | (a->n[1] & mask1);
Expand Down Expand Up @@ -570,8 +571,9 @@ 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;
volatile int vflag = flag;
SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
mask0 = flag + ~((uint64_t)0);
mask0 = vflag + ~((uint64_t)0);
mask1 = ~mask0;
r->n[0] = (r->n[0] & mask0) | (a->n[0] & mask1);
r->n[1] = (r->n[1] & mask0) | (a->n[1] & mask1);
Expand Down
3 changes: 2 additions & 1 deletion src/scalar_4x64_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -811,8 +811,9 @@ 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;
volatile int vflag = flag;
SECP256K1_CHECKMEM_CHECK_VERIFY(r->d, sizeof(r->d));
mask0 = flag + ~((uint64_t)0);
mask0 = vflag + ~((uint64_t)0);
mask1 = ~mask0;
r->d[0] = (r->d[0] & mask0) | (a->d[0] & mask1);
r->d[1] = (r->d[1] & mask0) | (a->d[1] & mask1);
Expand Down
3 changes: 2 additions & 1 deletion src/scalar_8x32_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -632,8 +632,9 @@ 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;
volatile int vflag = flag;
SECP256K1_CHECKMEM_CHECK_VERIFY(r->d, sizeof(r->d));
mask0 = flag + ~((uint32_t)0);
mask0 = vflag + ~((uint32_t)0);
mask1 = ~mask0;
r->d[0] = (r->d[0] & mask0) | (a->d[0] & mask1);
r->d[1] = (r->d[1] & mask0) | (a->d[1] & mask1);
Expand Down
3 changes: 2 additions & 1 deletion src/scalar_low_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,9 @@ 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;
volatile int vflag = flag;
SECP256K1_CHECKMEM_CHECK_VERIFY(r, sizeof(*r));
mask0 = flag + ~((uint32_t)0);
mask0 = vflag + ~((uint32_t)0);
mask1 = ~mask0;
*r = (*r & mask0) | (*a & mask1);
}
Expand Down

0 comments on commit 2d51a45

Please sign in to comment.