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.
This commit is contained in:
Sean Parkinson
2026-01-07 09:44:15 +10:00
parent 2354ea196b
commit 1aa79af41e
4 changed files with 27 additions and 17 deletions

View File

@@ -21208,13 +21208,11 @@ static byte MaskMac(const byte* data, int sz, int macSz, byte* expMac)
int i, j;
int r = 0;
unsigned char mac[WC_MAX_DIGEST_SIZE];
volatile int scanStart = sz - 1 - TLS_MAX_PAD_SZ - macSz;
int scanStart = sz - 1 - TLS_MAX_PAD_SZ - macSz;
volatile int macEnd = sz - 1 - data[sz - 1];
volatile int macStart = macEnd - macSz;
int macStart = macEnd - macSz;
volatile int maskScanStart;
volatile int maskMacStart;
volatile unsigned char started;
volatile unsigned char notEnded;
unsigned char good = 0;
maskScanStart = ctMaskIntGTE(scanStart, 0);
@@ -21224,22 +21222,31 @@ static byte MaskMac(const byte* data, int sz, int macSz, byte* expMac)
/* Div on Intel has different speeds depending on value.
* Use a bitwise AND or mod a specific value (converted to mul). */
if ((macSz & (macSz - 1)) == 0)
r = (macSz - (scanStart - macStart)) & (macSz - 1);
if ((macSz & (macSz - 1)) == 0) {
r = macSz - scanStart;
r += macStart;
r &= (macSz - 1);
}
#ifndef NO_SHA
else if (macSz == WC_SHA_DIGEST_SIZE)
r = (macSz - (scanStart - macStart)) % WC_SHA_DIGEST_SIZE;
else if (macSz == WC_SHA_DIGEST_SIZE) {
r = macSz - scanStart;
r += macStart;
r %= WC_SHA_DIGEST_SIZE;
}
#endif
#ifdef WOLFSSL_SHA384
else if (macSz == WC_SHA384_DIGEST_SIZE)
r = (macSz - (scanStart - macStart)) % WC_SHA384_DIGEST_SIZE;
else if (macSz == WC_SHA384_DIGEST_SIZE) {
r = macSz - scanStart;
r += macStart;
r %= WC_SHA384_DIGEST_SIZE;
}
#endif
XMEMSET(mac, 0, (size_t)(macSz));
for (i = scanStart; i < sz; i += macSz) {
for (j = 0; j < macSz && j + i < sz; j++) {
started = ctMaskGTE(i + j, macStart);
notEnded = ctMaskLT(i + j, macEnd);
unsigned char started = ctMaskGTE(i + j, macStart);
unsigned char notEnded = ctMaskLT(i + j, macEnd);
mac[j] |= started & notEnded & data[i + j];
}
}

View File

@@ -10350,7 +10350,8 @@ int WARN_UNUSED_RESULT AES_GCM_decrypt_C(
/* now use res as a mask for constant time return of ret, unless tag
* mismatch, whereupon AES_GCM_AUTH_E is returned.
*/
ret = (ret & ~res) | (res & WC_NO_ERR_TRACE(AES_GCM_AUTH_E));
ret = (ret & ~res);
ret |= (res & WC_NO_ERR_TRACE(AES_GCM_AUTH_E));
#endif
return ret;
}

View File

@@ -775,7 +775,9 @@ WC_MISC_STATIC WC_INLINE void ctMaskCopy(byte mask, byte* dst, byte* src,
#if !defined(WOLFSSL_NO_CT_OPS) && !defined(WOLFSSL_NO_CT_MAX_MIN) && \
defined(WORD64_AVAILABLE)
volatile word32 gte_mask = (word32)ctMaskWord32GTE(a, b);
return (a & ~gte_mask) | (b & gte_mask);
word32 r = (a & ~gte_mask);
r |= (b & gte_mask);
return r;
#else /* WOLFSSL_NO_CT_OPS */
return a > b ? b : a;
#endif /* WOLFSSL_NO_CT_OPS */

View File

@@ -18258,14 +18258,14 @@ int sp_to_unsigned_bin_len_ct(const sp_int* a, byte* out, int outSz)
/* Start at the end of the buffer - least significant byte. */
int j;
unsigned int i;
volatile sp_int_digit mask = (sp_int_digit)-1;
byte mask = (byte)-1;
sp_int_digit d;
/* Put each digit in. */
i = 0;
for (j = outSz - 1; j >= 0; ) {
unsigned int b;
volatile unsigned int notFull = (i < (unsigned int)a->used - 1);
volatile byte notFull = ctMaskLT((int)i, (int)a->used - 1);
d = a->dp[i];
/* Place each byte of a digit into the buffer. */
@@ -18273,7 +18273,7 @@ int sp_to_unsigned_bin_len_ct(const sp_int* a, byte* out, int outSz)
out[j--] = (byte)(d & mask);
d >>= 8;
}
mask &= (sp_int_digit)(-(int)notFull);
mask &= notFull;
i += (unsigned int)(1 & mask);
}
}