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.
That looks right to me, old code looks like a cut and paste error. Do you have a test case to verify the fix?
(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.
Group: mozilla-confidential, mozilla-corporation-confidential
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.
Group: mozilla-confidential, mozilla-corporation-confidential → core-security
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: 8 years ago
Resolution: --- → INVALID
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.
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+
(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.
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.
You need to log in before you can comment on or make changes to this bug.