From c58498b2dd95bac99cb3fa9014fa64cf12dd2f18 Mon Sep 17 00:00:00 2001 From: Ben Gamari Date: Thu, 29 Oct 2020 19:27:11 -0400 Subject: rts/linker: Fix relocation overflow in PE linker 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. (cherry picked from commit 6d21ecee535782f01dba9947a49e282afee25724) --- libraries/Cabal | 2 +- rts/linker/PEi386.c | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/libraries/Cabal b/libraries/Cabal index 8199c3f838..bd07f0a095 160000 --- a/libraries/Cabal +++ b/libraries/Cabal @@ -1 +1 @@ -Subproject commit 8199c3f838a15fb9b7c8d3527603084b2474d877 +Subproject commit bd07f0a095869b91a590d8a564f716a6a136818a diff --git a/rts/linker/PEi386.c b/rts/linker/PEi386.c index 0e41f34ce2..e0f61147af 100644 --- a/rts/linker/PEi386.c +++ b/rts/linker/PEi386.c @@ -1943,13 +1943,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); } @@ -1961,14 +1963,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); } -- cgit v1.2.1