Skip to content

Commit 0ed4934

Browse files
committed
Multiple volatile variables in a C statement undefined
Undefined behaviour when there are multiple volatile variables accessed in the one C statement. Changes to introduce non-volatile temporaries, split statement or make variable non-volatile.
1 parent 2354ea1 commit 0ed4934

File tree

4 files changed

+25
-16
lines changed

4 files changed

+25
-16
lines changed

src/internal.c

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21211,10 +21211,8 @@ static byte MaskMac(const byte* data, int sz, int macSz, byte* expMac)
2121121211
volatile int scanStart = sz - 1 - TLS_MAX_PAD_SZ - macSz;
2121221212
volatile int macEnd = sz - 1 - data[sz - 1];
2121321213
volatile int macStart = macEnd - macSz;
21214-
volatile int maskScanStart;
21215-
volatile int maskMacStart;
21216-
volatile unsigned char started;
21217-
volatile unsigned char notEnded;
21214+
int maskScanStart;
21215+
int maskMacStart;
2121821216
unsigned char good = 0;
2121921217

2122021218
maskScanStart = ctMaskIntGTE(scanStart, 0);
@@ -21224,22 +21222,31 @@ static byte MaskMac(const byte* data, int sz, int macSz, byte* expMac)
2122421222

2122521223
/* Div on Intel has different speeds depending on value.
2122621224
* Use a bitwise AND or mod a specific value (converted to mul). */
21227-
if ((macSz & (macSz - 1)) == 0)
21228-
r = (macSz - (scanStart - macStart)) & (macSz - 1);
21225+
if ((macSz & (macSz - 1)) == 0) {
21226+
r = macSz - scanStart;
21227+
r += macStart;
21228+
r &= (macSz - 1);
21229+
}
2122921230
#ifndef NO_SHA
21230-
else if (macSz == WC_SHA_DIGEST_SIZE)
21231-
r = (macSz - (scanStart - macStart)) % WC_SHA_DIGEST_SIZE;
21231+
else if (macSz == WC_SHA_DIGEST_SIZE) {
21232+
r = macSz - scanStart;
21233+
r += macStart;
21234+
r %= WC_SHA_DIGEST_SIZE;
21235+
}
2123221236
#endif
2123321237
#ifdef WOLFSSL_SHA384
21234-
else if (macSz == WC_SHA384_DIGEST_SIZE)
21235-
r = (macSz - (scanStart - macStart)) % WC_SHA384_DIGEST_SIZE;
21238+
else if (macSz == WC_SHA384_DIGEST_SIZE) {
21239+
r = macSz - scanStart;
21240+
r += macStart;
21241+
r %= WC_SHA384_DIGEST_SIZE;
21242+
}
2123621243
#endif
2123721244

2123821245
XMEMSET(mac, 0, (size_t)(macSz));
2123921246
for (i = scanStart; i < sz; i += macSz) {
2124021247
for (j = 0; j < macSz && j + i < sz; j++) {
21241-
started = ctMaskGTE(i + j, macStart);
21242-
notEnded = ctMaskLT(i + j, macEnd);
21248+
unsigned char started = ctMaskGTE(i + j, macStart);
21249+
unsigned char notEnded = ctMaskLT(i + j, macEnd);
2124321250
mac[j] |= started & notEnded & data[i + j];
2124421251
}
2124521252
}

wolfcrypt/src/aes.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10350,7 +10350,8 @@ int WARN_UNUSED_RESULT AES_GCM_decrypt_C(
1035010350
/* now use res as a mask for constant time return of ret, unless tag
1035110351
* mismatch, whereupon AES_GCM_AUTH_E is returned.
1035210352
*/
10353-
ret = (ret & ~res) | (res & WC_NO_ERR_TRACE(AES_GCM_AUTH_E));
10353+
ret = (ret & ~res);
10354+
ret |= (res & WC_NO_ERR_TRACE(AES_GCM_AUTH_E));
1035410355
#endif
1035510356
return ret;
1035610357
}

wolfcrypt/src/misc.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -775,7 +775,9 @@ WC_MISC_STATIC WC_INLINE void ctMaskCopy(byte mask, byte* dst, byte* src,
775775
#if !defined(WOLFSSL_NO_CT_OPS) && !defined(WOLFSSL_NO_CT_MAX_MIN) && \
776776
defined(WORD64_AVAILABLE)
777777
volatile word32 gte_mask = (word32)ctMaskWord32GTE(a, b);
778-
return (a & ~gte_mask) | (b & gte_mask);
778+
word32 r = (a & ~gte_mask);
779+
r |= (b & gte_mask);
780+
return r;
779781
#else /* WOLFSSL_NO_CT_OPS */
780782
return a > b ? b : a;
781783
#endif /* WOLFSSL_NO_CT_OPS */

wolfcrypt/src/sp_int.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18265,15 +18265,14 @@ int sp_to_unsigned_bin_len_ct(const sp_int* a, byte* out, int outSz)
1826518265
i = 0;
1826618266
for (j = outSz - 1; j >= 0; ) {
1826718267
unsigned int b;
18268-
volatile unsigned int notFull = (i < (unsigned int)a->used - 1);
1826918268

1827018269
d = a->dp[i];
1827118270
/* Place each byte of a digit into the buffer. */
1827218271
for (b = 0; (j >= 0) && (b < SP_WORD_SIZEOF); b++) {
1827318272
out[j--] = (byte)(d & mask);
1827418273
d >>= 8;
1827518274
}
18276-
mask &= (sp_int_digit)(-(int)notFull);
18275+
mask &= (sp_int_digit)(-(int)(i < (unsigned int)a->used - 1));
1827718276
i += (unsigned int)(1 & mask);
1827818277
}
1827918278
}

0 commit comments

Comments
 (0)