Closed
Bug 679397
Opened 13 years ago
Closed 13 years ago
X64 branch patch code seems to be wrong for jmp 64bit, but is actually fine: comment needed.
Categories
(Core Graveyard :: Nanojit, defect)
Tracking
(Not tracked)
RESOLVED
INVALID
People
(Reporter: shu, Unassigned)
Details
Attachments
(1 file, 1 obsolete file)
939 bytes,
patch
|
edwsmith
:
review+
|
Details | Diff | Splinter Review |
When patching jmp 64bit targets, we write the target 6 bytes from the start of the jmp. AIUI, jmp 64bit is FF/4, then the operand.
Reporter | ||
Comment 1•13 years ago
|
||
Comment 2•13 years ago
|
||
That looks right to me, old code looks like a cut and paste error. Do you have a test case to verify the fix?
Reporter | ||
Comment 3•13 years ago
|
||
(In reply to Edwin Smith from comment #2) > That looks right to me, old code looks like a cut and paste error. Do you > have a test case to verify the fix? I found this while trying to understand how nanojit patches jumps. I'm not even sure how to generate 64bit jumps.
Updated•13 years ago
|
Group: mozilla-confidential, mozilla-corporation-confidential
Reporter | ||
Comment 4•13 years ago
|
||
Edwin, I OOM trying to generate a 64bit jump on my Linux VM using the shell test from the fuzzer in bug 624439. Any suggestions on how to verify this fix? I also asked Dave to mark this bug confidential since it probably can be used to perform heap sprays -- if the user doesn't OOM first. I imagine this really needs to be marked a Security-Sensitive Core Bug, so if anyone can re-mark it as that, please do.
Reporter | ||
Updated•13 years ago
|
Attachment #553508 -
Flags: review?(edwsmith)
Updated•13 years ago
|
Group: mozilla-confidential, mozilla-corporation-confidential → core-security
Reporter | ||
Updated•13 years ago
|
Attachment #553508 -
Flags: review?(edwsmith)
Reporter | ||
Comment 5•13 years ago
|
||
I jumped the gun on this. Many thanks to Edwin for his help on this: 64bit jumps in nanojit x64 are encoded using RIP-relative addressing, so the first 4 bytes after FF 25 are the offset, which is kept as 0, so patch+6 is correct.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → INVALID
Comment 6•13 years ago
|
||
It still might make sense to add a comment about it in the code if it is not obvious to understand :) That way, other people looking at this will not run into this mistake again.
Reporter | ||
Comment 7•13 years ago
|
||
Attachment #553508 -
Attachment is obsolete: true
Comment 8•13 years ago
|
||
Comment on attachment 553869 [details] [diff] [review] Clarification comment I can push this to nanojit-central if you need someone to do it.
Attachment #553869 -
Flags: review+
Reporter | ||
Comment 9•13 years ago
|
||
(In reply to Edwin Smith from comment #8) > Comment on attachment 553869 [details] [diff] [review] > Clarification comment > > I can push this to nanojit-central if you need someone to do it. Please do! Thanks.
Updated•13 years ago
|
Summary: X64 branch patch code seems to be wrong for jmp 64bit → X64 branch patch code seems to be wrong for jmp 64bit, but is actually fine: comment needed.
Updated•12 years ago
|
Group: core-security
Assignee | ||
Updated•10 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•