Closed Bug 780936 Opened 12 years ago Closed 12 years ago

IonMonkey: Crash at weird address and !exploitable shows an Exploitable Data Execution Prevention Violation

Categories

(Core :: JavaScript Engine, defect)

Other Branch
defect
Not set
critical

Tracking

()

VERIFIED FIXED
Tracking Status
firefox15 --- unaffected
firefox16 --- unaffected
firefox17 --- unaffected
firefox-esr10 --- unaffected

People

(Reporter: gkw, Assigned: nbp)

References

Details

(4 keywords, Whiteboard: [ion:p1:fx18])

Attachments

(2 files, 4 obsolete files)

Attached file fragile testcase (obsolete) (deleted) —
The attached testcase crashes js debug and opt shells on IonMonkey changeset b2361e15b665 with --ion-eager and -a at 0x0000000001cfa3e7 on Windows 7. 0:000:x86> !exploitable -v HostMachine\HostUser Executing Processor Architecture is x86 Debuggee is in User Mode Debuggee is a live user mode debugging session on the local machine Event Type: Exception Exception Faulting Address: 0x1cfa3e7 First Chance Exception Type: STATUS_ACCESS_VIOLATION (0xC0000005) Exception Sub-Type: Data Execution Protection (DEP) Violation Exception Hash (Major/Minor): 0x6e05193a.0x7505193a Stack Trace: Unknown Instruction Address: 0x0000000001cfa3e7 Description: Data Execution Prevention Violation Short Description: DEPViolation Exploitability Classification: EXPLOITABLE Recommended Bug Title: Exploitable - Data Execution Prevention Violation starting at Unknown Symbol @ 0x0000000001cfa3e7 (Hash=0x6e05193a.0x7505193a) User mode DEP access violations are exploitable.
There is no stack even in Mac 64-bit shells: opt: Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_INVALID_ADDRESS at address: 0x0000000103514510 0x0000000103514510 in ?? () (gdb) bt #0 0x0000000103514510 in ?? () Cannot access memory at address 0x103514510 (gdb) x/i $pc 0x103514510: Cannot access memory at address 0x103514510 (gdb) debug: Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_INVALID_ADDRESS at address: 0x0000000104067720 0x0000000104067720 in ?? () (gdb) bt #0 0x0000000104067720 in ?? () Cannot access memory at address 0x104067720 (gdb)
Hardware: x86 → All
I manually bisected this after the vague crash pattern confused autobisect. The first bad revision is: changeset: 99584:fdc72b8c249c user: Jan de Mooij date: Fri Jul 06 11:17:34 2012 +0200 summary: Bug 767419 - Support idempotent GetProperty ICs. r=dvander,bhackett
Blocks: 767419
Whiteboard: [ion:p1:fx18]
Attached file Valgrind stack (deleted) —
Attached file slightly more reduced testcase (obsolete) (deleted) —
(thanks Nicolas for helping me out a little more here) Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_INVALID_ADDRESS at address: 0x00000001034518fc 0x00000001034518fc in ?? () (gdb) b EnterIon Breakpoint 1 at 0x1005751cc: file /Users/skywalker/Desktop/jsfunfuzz-ionmonkey-g7q_zK-b2361e15b665-103396/compilePath/js/src/ion/Ion.cpp, line 1096. (gdb) r The program being debugged has been started already. Start it from the beginning? (y or n) y Starting program: /Users/skywalker/Desktop/jsfunfuzz-ionmonkey-g7q_zK-b2361e15b665-103396/js-dbg-64-ionmonkey-darwin --ion-eager -a 169.js Breakpoint 1, EnterIon (cx=0x101115c20, fp=0x1012750b0, jitcode=0x101699f20) at /Users/skywalker/Desktop/jsfunfuzz-ionmonkey-g7q_zK-b2361e15b665-103396/compilePath/js/src/ion/Ion.cpp:1096 1096 JS_CHECK_RECURSION(cx, return IonExec_Error); (gdb) disable 1 (gdb) c Continuing. fuzzSeed: 104908815 Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_INVALID_ADDRESS at address: 0x00000001034518fc 0x00000001034518fc in ?? () (gdb) call js_DumpBacktrace(0x101115c20) #0 0x0 169.js:717 (0x102318970 @ 18) #1 0x0 169.js:712 (0x1023188c8 @ 91) #2 0x0 169.js:727 (0x102318c10 @ 15) #3 0x0 169.js:717 (0x102318970 @ 18) #4 0x0 169.js:712 (0x1023188c8 @ 91) #5 0x0 169.js:723 (0x102318b68 @ 7) #6 0x0 169.js:712 (0x1023188c8 @ 91) #7 0x0 169.js:720 (0x102318a18 @ 7) #8 0x0 169.js:712 (0x1023188c8 @ 91) #9 0x0 169.js:727 (0x102318c10 @ 15) #10 0x0 169.js:717 (0x102318970 @ 18) #11 0x0 169.js:712 (0x1023188c8 @ 91) #12 0x0 169.js:720 (0x102318a18 @ 7) #13 0x0 169.js:712 (0x1023188c8 @ 91) #14 0x0 169.js:720 (0x102318a18 @ 7) #15 0x0 169.js:712 (0x1023188c8 @ 91) #16 0x0 169.js:723 (0x102318b68 @ 7) #17 0x0 169.js:712 (0x1023188c8 @ 91) #18 0x0 169.js:727 (0x102318c10 @ 15) #19 0x0 169.js:717 (0x102318970 @ 18) #20 0x0 169.js:712 (0x1023188c8 @ 91) #21 0x0 169.js:720 (0x102318a18 @ 7) #22 0x0 169.js:712 (0x1023188c8 @ 91) #23 0x0 169.js:720 (0x102318a18 @ 7) #24 0x0 169.js:712 (0x1023188c8 @ 91) #25 0x0 169.js:720 (0x102318a18 @ 7) #26 0x0 169.js:712 (0x1023188c8 @ 91) #27 0x0 169.js:727 (0x102318c10 @ 15) #28 0x0 169.js:717 (0x102318970 @ 18) #29 0x0 169.js:712 (0x1023188c8 @ 91) #30 0x101275770 169.js:723 (0x102318b68 @ 7) #31 0x1012756f0 169.js:712 (0x1023188c8 @ 91) #32 0x101275660 169.js:727 (0x102318c10 @ 15) #33 0x1012755e0 169.js:717 (0x102318970 @ 18) #34 0x101275558 169.js:712 (0x1023188c8 @ 91) #35 0x1012754c8 169.js:720 (0x102318a18 @ 7) #36 0x101275448 169.js:712 (0x1023188c8 @ 91) #37 0x1012753b8 169.js:706 (0x102318778 @ 48) #38 0x0 169.js:675 (0x102318388 @ 42) #39 0x101275288 169.js:283 (0x102315580 @ 67) #40 0x1012751e0 169.js:229 (0x102315190 @ 157) #41 0x101275150 169.js:655 (0x102318040 @ 28) #42 0x1012750c0 169.js:220 (0x102315190 @ 40) #43 0x101275030 169.js:732 (0x1023070e8 @ 2344) warning: Unable to restore previously selected frame. (gdb)
Attachment #649708 - Attachment is obsolete: true
Assignee: general → nicolas.b.pierron
Status: NEW → ASSIGNED
Attached file Aggresively reduced testcase (deleted) —
Attachment #649866 - Attachment is obsolete: true
This issue sounds like a marking issue of the Invalidation path. What happens after we are compiling multiple times and did multiple bailouts: - call & compile. - invalidate all ion code. - return into ThunkToInterpreter. - return in the argument rectifier. - return in the Ion Code of the parent frame and follow the jump added by the invalidation. - SEGV because the memory is no longer readable.
CodeGenerator::visitCallGeneric can generate the following sequence of instruction 0x00007ffff7f8af60: … 0x00007ffff7f8af63: callq 0x7ffff7f88008 0x00007ffff7f8af68: jmpq 0x7ffff7f8af6f 0x00007ffff7f8af6d: callq *%rbx 0x00007ffff7f8af6f: add $0x8,%rsp 0x00007ffff7f8af73: … The issue being that during an invalidation the invalidation path assume that the call-site will never be used and used the call-site to store the displacement to the invalidation epilogue. Unfortunately, when both calls are used and that the script got invalidated both call-site are patched with 4 bytes of data. This patch damage the jump located at 0x00007ffff7f8af68 and makes it jump in unmapped memory. The current patch just add extra unused instructions after the jump between the 2 calls such as patch of the second call-site does not damage the jump needed to join the OSIPoint located after the LCallGeneric. Still, I wonder where this call-site patch is needed, any ideas?
Attachment #651093 - Flags: review?(dvander)
Comment on attachment 651093 [details] [diff] [review] Invalidation, Add space to avoid damaging other return paths. Good catch... these are tricky and thus I'm a little reluctant to spot fix it here, in this way. It seems like instead, calls which may be invalidated should store the current assembler position. Then, at each call, we should either: (1) Automatically insert nops as padding if the delta isn't high enough, or (2) Assert that there is enough space and include a function to calculate the correct padding and insert nops. I actually rather like #2 as then we could replace the logic at the end of CG*::generateInvalidateEpilogue(), and avoid nops being inserted after the "bind(&rejoin)".
Attachment #651093 - Attachment is obsolete: true
Attachment #652654 - Flags: review?(dvander)
Attached patch Move code and fix bug. (obsolete) (deleted) — Splinter Review
Attachment #654036 - Flags: review?(dvander)
This bug should not longer reproduce with Bug 784568's patch.
(In reply to Nicolas B. Pierron [:pierron] [:nbp] from comment #11) > This bug should not longer reproduce with Bug 784568's patch. Verifying that this does not reproduce anymore on Windows 7 with IonMonkey changeset f9ff9c554d4b. Resolving FIXED by bug 784568. The aggressively-reduced testcase in comment 5 can be checked in eventually... when it's safe. Setting the in-testsuite? flag.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Comment on attachment 654036 [details] [diff] [review] Move code and fix bug. Nicolas says this is obsolete, canceling review.
Attachment #654036 - Attachment is obsolete: true
Attachment #654036 - Flags: review?(dvander)
Attachment #652654 - Attachment is obsolete: true
Attachment #652654 - Attachment is private: true
Attachment #652654 - Flags: review?(dvander)
IonMonkey bug,marking esr10 unaffected
Group: core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: