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)
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)
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.
Reporter | ||
Comment 1•12 years ago
|
||
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
Reporter | ||
Comment 2•12 years ago
|
||
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
Updated•12 years ago
|
Whiteboard: [ion:p1:fx18]
Reporter | ||
Comment 3•12 years ago
|
||
Reporter | ||
Comment 4•12 years ago
|
||
(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
Reporter | ||
Updated•12 years ago
|
Keywords: sec-critical
Assignee | ||
Updated•12 years ago
|
Assignee: general → nicolas.b.pierron
Status: NEW → ASSIGNED
Assignee | ||
Comment 5•12 years ago
|
||
Attachment #649866 -
Attachment is obsolete: true
Updated•12 years ago
|
status-firefox15:
--- → unaffected
status-firefox16:
--- → unaffected
status-firefox17:
--- → unaffected
Assignee | ||
Comment 6•12 years ago
|
||
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.
Assignee | ||
Comment 7•12 years ago
|
||
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)".
Updated•12 years ago
|
Attachment #651093 -
Flags: review?(dvander)
Assignee | ||
Updated•12 years ago
|
Attachment #651093 -
Attachment is obsolete: true
Attachment #652654 -
Flags: review?(dvander)
Assignee | ||
Comment 9•12 years ago
|
||
Attachment #654036 -
Flags: review?(dvander)
Assignee | ||
Comment 10•12 years ago
|
||
This bug should not longer reproduce with Bug 784568's patch.
Reporter | ||
Comment 11•12 years ago
|
||
(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
Updated•12 years ago
|
Status: RESOLVED → VERIFIED
Comment 12•12 years ago
|
||
JSBugMon: This bug has been automatically verified fixed.
Reporter | ||
Comment 13•12 years ago
|
||
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)
Reporter | ||
Updated•12 years ago
|
Attachment #652654 -
Attachment is obsolete: true
Attachment #652654 -
Attachment is private: true
Attachment #652654 -
Flags: review?(dvander)
Comment 14•12 years ago
|
||
IonMonkey bug,marking esr10 unaffected
status-firefox-esr10:
--- → unaffected
Updated•9 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•