1314183 - TestDllInterceptor crash relocating an IP-relative instruction

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[(Away)]

With Win64 ASan, TestDllInterceptor!rotatePayload contains this instruction:

00007ff7`bb52100a 488b05ff7a0000  mov     rax,qword ptr [TestDllInterceptor!__asan_shadow_memory_dynamic_address (00007ff7`bb528b10)]

The address of __asan_shadow_memory_dynamic_address is given as IP-relative. CreateTrampoline and CountModRmSib accept this instruction and relocate it, without fixup, leading to a crash.

Since there's no code in CreateTrampoline that does fixup on anything but jumps, is there any reason why CountModRmSib should accept IP-relative instructions?

Comment 1

[(Away)]

(Just to be clear, I'd be totally fine with CreateTrampoline simply rejecting this sequence; I can deal with the test failure. I don't think we need to properly relocate this instruction unless "real" code needs it.)

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

Good catch. Patch forthcoming, I'm testing it right now.

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1e8d855cc08b

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Bug 1314183: Ensure that nsWindowsDllInterceptor does not accept RIP-relative displacements on amd64;

Review commit: https://reviewboard.mozilla.org/r/89928/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89928/

Comment 5

[(Away)]

Comment on attachment 8806534 [details]
Bug 1314183: Ensure that nsWindowsDllInterceptor does not accept RIP-relative displacements on amd64;

https://reviewboard.mozilla.org/r/89928/#review89440

::: xpcom/build/nsWindowsDllInterceptor.h:520
(Diff revision 1)
>        case kModNoRegDisp:
> -        if ((*aModRm & kMaskRm) == kRmNoRegDispDisp32 ||
> -            ((*aModRm & kMaskRm) == kRmNeedSib &&
> +        if ((*aModRm & kMaskRm) == kRmNoRegDispDisp32) {
> +#if defined(_M_X64)
> +          // RIP-relative on amd64, currently unsupported
> +          return -1;
> +#else

It would be good to mention that the x86 equivalent is ok because it uses an absolute address.

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 8806534 [details]
Bug 1314183: Ensure that nsWindowsDllInterceptor does not accept RIP-relative displacements on amd64;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89928/diff/1-2/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e7e0a189b16a
Ensure that nsWindowsDllInterceptor does not accept RIP-relative displacements on amd64; r=dmajor

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e7e0a189b16a