Skip to content

Commit 4a496a3

Browse files
ct: Use volatile "trick" in all fe/scalar cmov implementations
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 #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 #864 (comment)
1 parent 464a911 commit 4a496a3

5 files changed

+14
-7
lines changed

src/field_10x26_impl.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -1147,8 +1147,9 @@ static void secp256k1_fe_sqr(secp256k1_fe *r, const secp256k1_fe *a) {
11471147

11481148
static SECP256K1_INLINE void secp256k1_fe_cmov(secp256k1_fe *r, const secp256k1_fe *a, int flag) {
11491149
uint32_t mask0, mask1;
1150+
volatile int vflag = flag;
11501151
SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
1151-
mask0 = flag + ~((uint32_t)0);
1152+
mask0 = vflag + ~((uint32_t)0);
11521153
mask1 = ~mask0;
11531154
r->n[0] = (r->n[0] & mask0) | (a->n[0] & mask1);
11541155
r->n[1] = (r->n[1] & mask0) | (a->n[1] & mask1);
@@ -1246,8 +1247,9 @@ static SECP256K1_INLINE void secp256k1_fe_half(secp256k1_fe *r) {
12461247

12471248
static SECP256K1_INLINE void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, const secp256k1_fe_storage *a, int flag) {
12481249
uint32_t mask0, mask1;
1250+
volatile int vflag = flag;
12491251
SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
1250-
mask0 = flag + ~((uint32_t)0);
1252+
mask0 = vflag + ~((uint32_t)0);
12511253
mask1 = ~mask0;
12521254
r->n[0] = (r->n[0] & mask0) | (a->n[0] & mask1);
12531255
r->n[1] = (r->n[1] & mask0) | (a->n[1] & mask1);

src/field_5x52_impl.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -487,8 +487,9 @@ static void secp256k1_fe_sqr(secp256k1_fe *r, const secp256k1_fe *a) {
487487

488488
static SECP256K1_INLINE void secp256k1_fe_cmov(secp256k1_fe *r, const secp256k1_fe *a, int flag) {
489489
uint64_t mask0, mask1;
490+
volatile int vflag = flag;
490491
SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
491-
mask0 = flag + ~((uint64_t)0);
492+
mask0 = vflag + ~((uint64_t)0);
492493
mask1 = ~mask0;
493494
r->n[0] = (r->n[0] & mask0) | (a->n[0] & mask1);
494495
r->n[1] = (r->n[1] & mask0) | (a->n[1] & mask1);
@@ -570,8 +571,9 @@ static SECP256K1_INLINE void secp256k1_fe_half(secp256k1_fe *r) {
570571

571572
static SECP256K1_INLINE void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, const secp256k1_fe_storage *a, int flag) {
572573
uint64_t mask0, mask1;
574+
volatile int vflag = flag;
573575
SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n));
574-
mask0 = flag + ~((uint64_t)0);
576+
mask0 = vflag + ~((uint64_t)0);
575577
mask1 = ~mask0;
576578
r->n[0] = (r->n[0] & mask0) | (a->n[0] & mask1);
577579
r->n[1] = (r->n[1] & mask0) | (a->n[1] & mask1);

src/scalar_4x64_impl.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -811,8 +811,9 @@ SECP256K1_INLINE static void secp256k1_scalar_mul_shift_var(secp256k1_scalar *r,
811811

812812
static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) {
813813
uint64_t mask0, mask1;
814+
volatile int vflag = flag;
814815
SECP256K1_CHECKMEM_CHECK_VERIFY(r->d, sizeof(r->d));
815-
mask0 = flag + ~((uint64_t)0);
816+
mask0 = vflag + ~((uint64_t)0);
816817
mask1 = ~mask0;
817818
r->d[0] = (r->d[0] & mask0) | (a->d[0] & mask1);
818819
r->d[1] = (r->d[1] & mask0) | (a->d[1] & mask1);

src/scalar_8x32_impl.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -632,8 +632,9 @@ SECP256K1_INLINE static void secp256k1_scalar_mul_shift_var(secp256k1_scalar *r,
632632

633633
static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) {
634634
uint32_t mask0, mask1;
635+
volatile int vflag = flag;
635636
SECP256K1_CHECKMEM_CHECK_VERIFY(r->d, sizeof(r->d));
636-
mask0 = flag + ~((uint32_t)0);
637+
mask0 = vflag + ~((uint32_t)0);
637638
mask1 = ~mask0;
638639
r->d[0] = (r->d[0] & mask0) | (a->d[0] & mask1);
639640
r->d[1] = (r->d[1] & mask0) | (a->d[1] & mask1);

src/scalar_low_impl.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,9 @@ SECP256K1_INLINE static int secp256k1_scalar_eq(const secp256k1_scalar *a, const
116116

117117
static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) {
118118
uint32_t mask0, mask1;
119+
volatile int vflag = flag;
119120
SECP256K1_CHECKMEM_CHECK_VERIFY(r, sizeof(*r));
120-
mask0 = flag + ~((uint32_t)0);
121+
mask0 = vflag + ~((uint32_t)0);
121122
mask1 = ~mask0;
122123
*r = (*r & mask0) | (*a & mask1);
123124
}

0 commit comments

Comments
 (0)