From e91d142c49f0ff129a087f2b66380bd7c5da0617 Mon Sep 17 00:00:00 2001 Message-Id: From: Mark Wooding Date: Tue, 30 Oct 2018 22:05:18 +0000 Subject: [PATCH] progs/..., symm/...: Fix 32-bit right-shift idiom. Organization: Straylight/Edgeware From: Mark Wooding This one has a long and troubled history. Writing x >> 32 is undefined behaviour if x is only 32 bits wide. On the other hand, if it's /not/, then this is necessary to get hold of the upper bits. The obvious escape plan is to write (x >> 16) >> 16 (the parentheses are unfortunately necessary), but some Microsoft compilers managed do bungle compiling this: they merged the two shifts together and then decided that a shift by 32 places was a no-op. So I wrote ((x&~MASK32) >> 16) >> 16 which stood for many years. Unfortunately this is really wrong too: if x is wider than 32 bits, that's nice, but MASK32 /isn't/ necessarily, so ~MASK32 is all-bits zero and the high bits of x are just lost. Fix this by casting MASK32 to the-type-of-x before inverting it. Ugh. --- progs/cookie.c | 5 +++-- progs/dsig.c | 6 ++++-- symm/blkc.h | 4 ++-- symm/has160.c | 2 +- symm/hash.h | 4 ++-- symm/mars-mktab.c | 4 ++-- symm/md4.c | 2 +- symm/md5.c | 2 +- symm/rmd128.c | 2 +- symm/rmd160.c | 2 +- symm/rmd256.c | 2 +- symm/rmd320.c | 2 +- symm/sha.c | 2 +- symm/sha256.c | 2 +- symm/sha3.c | 4 ++-- symm/sha512.c | 2 +- symm/tiger.c | 2 +- symm/whirlpool.c | 2 +- 18 files changed, 27 insertions(+), 24 deletions(-) diff --git a/progs/cookie.c b/progs/cookie.c index 6239eb0c..c6912ffd 100644 --- a/progs/cookie.c +++ b/progs/cookie.c @@ -80,7 +80,7 @@ typedef struct cookie { octet *_p = (octet *)(p); \ const cookie *_c = (c); \ STORE32(_p + 0, _c->k); \ - STORE32(_p + 4, ((_c->exp & ~MASK32) >> 16) >> 16); \ + STORE32(_p + 4, ((_c->exp & ~(unsigned long)MASK32) >> 16) >> 16); \ STORE32(_p + 8, _c->exp); \ } while (0) @@ -97,7 +97,8 @@ typedef struct cookie { cookie *_c = (c); \ const octet *_p = (const octet *)(p); \ _c->k = LOAD32(_p + 0); \ - _c->exp = ((time_t)(((LOAD32(_p + 4) << 16) << 16) & ~MASK32) | \ + _c->exp = ((time_t)(((LOAD32(_p + 4) << 16) << 16) & \ + ~(unsigned long)MASK32) | \ (time_t)LOAD32(_p + 8)); \ } while (0) diff --git a/progs/dsig.c b/progs/dsig.c index 5e5a3cfe..1377bcaa 100644 --- a/progs/dsig.c +++ b/progs/dsig.c @@ -233,7 +233,8 @@ static int bget(block *b, FILE *fp, unsigned bin) octet buf[8]; if (fread(buf, sizeof(buf), 1, fp) < 1) return (E_EOF); - b->t = ((time_t)(((LOAD32(buf + 0) << 16) << 16) & ~MASK32) | + b->t = ((time_t)(((LOAD32(buf + 0) << 16) << 16) & + ~(unsigned long)MASK32) | (time_t)LOAD32(buf + 4)); } else { if (getstring(fp, &b->d, GSF_FILE)) @@ -325,7 +326,8 @@ static void blob(block *b, dstr *d) STORE32(d->buf + d->len, 0xffffffff); STORE32(d->buf + d->len + 4, 0xffffffff); } else { - STORE32(d->buf + d->len, ((b->t & ~MASK32) >> 16) >> 16); + STORE32(d->buf + d->len, + ((b->t & ~(unsigned long)MASK32) >> 16) >> 16); STORE32(d->buf + d->len + 4, b->t); } d->len += 8; diff --git a/symm/blkc.h b/symm/blkc.h index 1cbe729c..ff631f09 100644 --- a/symm/blkc.h +++ b/symm/blkc.h @@ -174,7 +174,7 @@ unsigned _i; BLKC_W(w); unsigned long _x = x; \ for (_i = 0; _i < PRE##_BLKSZ / 4; _i++) { \ *_w++ = U32(_x); \ - _x = ((_x & ~MASK32) >> 16) >> 16; \ + _x = ((_x & ~(unsigned long)MASK32) >> 16) >> 16; \ } \ } while (0) @@ -182,7 +182,7 @@ unsigned _i; BLKC_W(w); unsigned long _x = x; _w += PRE##_BLKSZ / 4; \ for (_i = 0; _i < PRE##_BLKSZ / 4; _i++) { \ *--_w = U32(_x); \ - _x = ((_x & ~MASK32) >> 16) >> 16; \ + _x = ((_x & ~(unsigned long)MASK32) >> 16) >> 16; \ } \ } while (0) diff --git a/symm/has160.c b/symm/has160.c index 483b9fe8..0fbaf488 100644 --- a/symm/has160.c +++ b/symm/has160.c @@ -172,7 +172,7 @@ void has160_set(has160_ctx *ctx, const void *buf, unsigned long count) ctx->e = LOAD32_L(p + 16); ctx->off = 0; ctx->nl = U32(count); - ctx->nh = U32(((count & ~MASK32) >> 16) >> 16); + ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16); } /* --- @has160_hash@ --- * diff --git a/symm/hash.h b/symm/hash.h index dc9ad024..eb3cd75e 100644 --- a/symm/hash.h +++ b/symm/hash.h @@ -68,10 +68,10 @@ \ { \ uint32 _l = U32(_bsz); \ - uint32 _h = ((_bsz & ~MASK32) >> 16) >> 16; \ + uint32 _h = ((_bsz & ~(size_t)MASK32) >> 16) >> 16; \ _bctx->nh += _h; \ _bctx->nl += _l; \ - if (_bctx->nl < _l || _bctx->nl & ~MASK32) \ + if (_bctx->nl < _l || _bctx->nl & ~(uint32)MASK32) \ _bctx->nh++; \ } \ \ diff --git a/symm/mars-mktab.c b/symm/mars-mktab.c index d43f7c07..53ce0f71 100644 --- a/symm/mars-mktab.c +++ b/symm/mars-mktab.c @@ -154,10 +154,10 @@ void sha_hash(sha_ctx *ctx, const void *buf, size_t sz) { uint32 _l = ((uint32) ((_bsz) & MASK32)); - uint32 _h = ((_bsz & ~MASK32) >> 16) >> 16; + uint32 _h = ((_bsz & ~(size_t)MASK32) >> 16) >> 16; _bctx->nh += _h; _bctx->nl += _l; - if (_bctx->nl < _l || _bctx->nl & ~MASK32) + if (_bctx->nl < _l || _bctx->nl & ~(uint32)MASK32) _bctx->nh++; } if (_bctx->off + _bsz < SHA_BUFSZ) { diff --git a/symm/md4.c b/symm/md4.c index eee5d6b3..15fb7503 100644 --- a/symm/md4.c +++ b/symm/md4.c @@ -185,7 +185,7 @@ void md4_set(md4_ctx *ctx, const void *buf, unsigned long count) ctx->d = LOAD32_L(p + 12); ctx->off = 0; ctx->nl = U32(count); - ctx->nh = U32(((count & ~MASK32) >> 16) >> 16); + ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16); } /* --- @md4_hash@ --- * diff --git a/symm/md5.c b/symm/md5.c index f3b37e19..cf6b3355 100644 --- a/symm/md5.c +++ b/symm/md5.c @@ -204,7 +204,7 @@ void md5_set(md5_ctx *ctx, const void *buf, unsigned long count) ctx->d = LOAD32_L(p + 12); ctx->off = 0; ctx->nl = U32(count); - ctx->nh = U32(((count & ~MASK32) >> 16) >> 16); + ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16); } /* --- @md5_hash@ --- * diff --git a/symm/rmd128.c b/symm/rmd128.c index 85b6c33e..606d0e57 100644 --- a/symm/rmd128.c +++ b/symm/rmd128.c @@ -284,7 +284,7 @@ void rmd128_set(rmd128_ctx *ctx, const void *buf, unsigned long count) ctx->d = LOAD32_L(p + 12); ctx->off = 0; ctx->nl = U32(count); - ctx->nh = U32(((count & ~MASK32) >> 16) >> 16); + ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16); } /* --- @rmd128_hash@ --- * diff --git a/symm/rmd160.c b/symm/rmd160.c index bc7e8672..3dbb5210 100644 --- a/symm/rmd160.c +++ b/symm/rmd160.c @@ -325,7 +325,7 @@ void rmd160_set(rmd160_ctx *ctx, const void *buf, unsigned long count) ctx->e = LOAD32_L(p + 16); ctx->off = 0; ctx->nl = U32(count); - ctx->nh = U32(((count & ~MASK32) >> 16) >> 16); + ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16); } /* --- @rmd160_hash@ --- * diff --git a/symm/rmd256.c b/symm/rmd256.c index 99648f5f..0a03065b 100644 --- a/symm/rmd256.c +++ b/symm/rmd256.c @@ -294,7 +294,7 @@ void rmd256_set(rmd256_ctx *ctx, const void *buf, unsigned long count) ctx->D = LOAD32_L(p + 28); ctx->off = 0; ctx->nl = U32(count); - ctx->nh = U32(((count & ~MASK32) >> 16) >> 16); + ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16); } /* --- @rmd256_hash@ --- * diff --git a/symm/rmd320.c b/symm/rmd320.c index 022903e7..0ccd790e 100644 --- a/symm/rmd320.c +++ b/symm/rmd320.c @@ -340,7 +340,7 @@ void rmd320_set(rmd320_ctx *ctx, const void *buf, unsigned long count) ctx->E = LOAD32_L(p + 36); ctx->off = 0; ctx->nl = U32(count); - ctx->nh = U32(((count & ~MASK32) >> 16) >> 16); + ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16); } /* --- @rmd320_hash@ --- * diff --git a/symm/sha.c b/symm/sha.c index 980fe802..35207273 100644 --- a/symm/sha.c +++ b/symm/sha.c @@ -210,7 +210,7 @@ void sha_set(sha_ctx *ctx, const void *buf, unsigned long count) ctx->e = LOAD32(p + 16); ctx->off = 0; ctx->nl = U32(count); - ctx->nh = U32(((count & ~MASK32) >> 16) >> 16); + ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16); } /* --- @sha_hash@ --- * diff --git a/symm/sha256.c b/symm/sha256.c index 5de3966d..7f91ff74 100644 --- a/symm/sha256.c +++ b/symm/sha256.c @@ -212,7 +212,7 @@ void sha256_set(sha256_ctx *ctx, const void *buf, unsigned long count) ctx->h = LOAD32(p + 28); ctx->off = 0; ctx->nl = U32(count); - ctx->nh = U32(((count & ~MASK32) >> 16) >> 16); + ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16); } /* --- @sha256_hash@, @sha224_hash@ --- * diff --git a/symm/sha3.c b/symm/sha3.c index 97a3f7a8..16967629 100644 --- a/symm/sha3.c +++ b/symm/sha3.c @@ -231,7 +231,7 @@ static void leftenc_sz(shake_ctx *ctx, size_t n) octet b[9]; unsigned i; - SET64(t, ((n&~MASK32) >> 16) >> 16, n&MASK32); + SET64(t, ((n&~(size_t)MASK32) >> 16) >> 16, n&MASK32); STORE64_B_(b + 1, t); for (i = 1; i < 8 && !b[i]; i++); i--; b[i] = 8 - i; @@ -244,7 +244,7 @@ static void rightenc_sz(shake_ctx *ctx, size_t n) octet b[9]; unsigned i; - SET64(t, ((n&~MASK32) >> 16) >> 16, n&MASK32); + SET64(t, ((n&~(size_t)MASK32) >> 16) >> 16, n&MASK32); STORE64_B_(b, t); for (i = 0; i < 7 && !b[i]; i++); b[8] = 8 - i; diff --git a/symm/sha512.c b/symm/sha512.c index a9a5180c..5c75eb8e 100644 --- a/symm/sha512.c +++ b/symm/sha512.c @@ -275,7 +275,7 @@ void sha512_set(sha512_ctx *ctx, const void *buf, unsigned long count) LOAD64_(ctx->h, p + 56); ctx->off = 0; ctx->nl = U32(count); - ctx->nh = U32(((count & ~MASK32) >> 16) >> 16); + ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16); } /* --- @sha512_hash@, @sha384_hash@ --- * diff --git a/symm/tiger.c b/symm/tiger.c index 95faf56f..48896602 100644 --- a/symm/tiger.c +++ b/symm/tiger.c @@ -99,7 +99,7 @@ void tiger_set(tiger_ctx *ctx, const void *buf, unsigned long count) LOAD64_L_(ctx->c, p + 16); ctx->off = 0; ctx->nl = U32(count); - ctx->nh = U32(((count & ~MASK32) >> 16) >> 16); + ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16); } /* --- @tiger_hash@ --- * diff --git a/symm/whirlpool.c b/symm/whirlpool.c index 708e3fce..f22a366a 100644 --- a/symm/whirlpool.c +++ b/symm/whirlpool.c @@ -206,7 +206,7 @@ void whirlpool_set(whirlpool_ctx *ctx, const void *buf, unsigned long count) } ctx->off = 0; ctx->nl = U32(count); - ctx->nh = U32(((count & ~MASK32) >> 16) >> 16); + ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16); } /* --- @whirlpool_hash@, @whirlpool256_hash@ --- * -- [mdw]