diff options
| author | Ben Gamari <ben@smart-cactus.org> | 2020-10-29 19:27:11 -0400 |
|---|---|---|
| committer | Ben Gamari <ben@smart-cactus.org> | 2020-10-31 13:34:45 -0400 |
| commit | 6d21ecee535782f01dba9947a49e282afee25724 (patch) | |
| tree | ed6190c3d42b6072ebb5862b04740d751caf9fbf | |
| parent | eb368078204bceba51e3c80e387ce1d776762cfd (diff) | |
| download | haskell-wip/T15808.tar.gz | |
rts/linker: Fix relocation overflow in PE linkerwip/T15808
Previously the overflow check for the IMAGE_REL_AMD64_ADDR32NB
relocation failed to account for the signed nature of the value.
Specifically, the overflow check was:
uint64_t v;
v = S + A;
if (v >> 32) { ... }
However, `v` ultimately needs to fit into 32-bits as a signed value.
Consequently, values `v > 2^31` in fact overflow yet this is not caught
by the existing overflow check.
Here we rewrite the overflow check to rather ensure that
`INT32_MIN <= v <= INT32_MAX`. There is now quite a bit of repetition
between the `IMAGE_REL_AMD64_REL32` and `IMAGE_REL_AMD64_ADDR32` cases
but I am leaving fixing this for future work.
This bug was first noticed by @awson.
Fixes #15808.
| -rw-r--r-- | rts/linker/PEi386.c | 10 |
1 files changed, 6 insertions, 4 deletions
diff --git a/rts/linker/PEi386.c b/rts/linker/PEi386.c index d7fef79afb..66bd22e8e9 100644 --- a/rts/linker/PEi386.c +++ b/rts/linker/PEi386.c @@ -1952,13 +1952,15 @@ ocResolve_PEi386 ( ObjectCode* oc ) { uint64_t v; v = S + A; - if (v >> 32) { + // N.B. in the case of the sign-extended relocations we must ensure that v + // fits in a signed 32-bit value. See #15808. + if (((int64_t) v > (int64_t) INT32_MAX) || ((int64_t) v < (int64_t) INT32_MIN)) { copyName (getSymShortName (info, sym), oc, symbol, sizeof(symbol)-1); S = makeSymbolExtra_PEi386(oc, symIndex, S, (char *)symbol); /* And retry */ v = S + A; - if (v >> 32) { + if (((int64_t) v > (int64_t) INT32_MAX) || ((int64_t) v < (int64_t) INT32_MIN)) { barf("IMAGE_REL_AMD64_ADDR32[NB]: High bits are set in %zx for %s", v, (char *)symbol); } @@ -1970,14 +1972,14 @@ ocResolve_PEi386 ( ObjectCode* oc ) { intptr_t v; v = S + (int32_t)A - ((intptr_t)pP) - 4; - if ((v > (intptr_t) INT32_MAX) || (v < (intptr_t) INT32_MIN)) { + if ((v > (int64_t) INT32_MAX) || (v < (int64_t) INT32_MIN)) { /* Make the trampoline then */ copyName (getSymShortName (info, sym), oc, symbol, sizeof(symbol)-1); S = makeSymbolExtra_PEi386(oc, symIndex, S, (char *)symbol); /* And retry */ v = S + (int32_t)A - ((intptr_t)pP) - 4; - if ((v > (intptr_t) INT32_MAX) || (v < (intptr_t) INT32_MIN)) { + if ((v > (int64_t) INT32_MAX) || (v < (int64_t) INT32_MIN)) { barf("IMAGE_REL_AMD64_REL32: High bits are set in %zx for %s", v, (char *)symbol); } |