Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix the wrong target address of direct/conditional x86 JUMPs (jmp, je..) #323

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

TartaRikker
Copy link

I am not sure that this bug is present in the other architectures, but in x86 if you dont supplied keystone with the correct base address(the address of the first instruction) when you use it as a jit assembler, then you will end up with some wrong jump address in your assembled code.
Your documentation says that base address can be ignored which is completely wrong and this caused me to spend so many hours messing with the core code trying to figure out where the bug is... though I'm fairly new to keystone and completely unfamiliar with the LLVM, I managed to apply some solutions to calculate the target address but it was just so easy to just do what I'm doing in the PR.
in fact you can't calculate target address if you didn't know the address of the jump instruction

@aquynh
Copy link
Member

aquynh commented Nov 1, 2017

unfortunately, this breaks all the CIs, for example https://travis-ci.org/keystone-engine/keystone/builds/295542838?utm_source=github_status&utm_medium=notification. can you fix that?

@TartaRikker
Copy link
Author

Sure

Fix syntax error and make it able to digest 'goto' statement
@TartaRikker
Copy link
Author

Sorry for that syntax error, I've used git web editor to make those changes which is inefficient. I did it so that I finish quickly but seems I stuck lol

@aquynh
Copy link
Member

aquynh commented Nov 29, 2017

sorry for the late reply, but do you have any examples with wrong output?

@TartaRikker
Copy link
Author

You can easily reproduce this by disassembling an already assembled code that contains a jump instruction and then assemble it again, you'll see that wrong address.

Example:

mov	dword ptr [esi + 0x6c], eax
test	eax, eax
jne	0x1011b930 <----- Note this
mov	eax, dword ptr  [0x101f3458]

The same code assembled with Keystone:

mov	dword ptr [esi + 0x6c], eax
test	eax, eax
jne	0x11029478 <------ Wrong address
mov	eax, dword ptr [0x101f3458]

By using this method of testing I managed to find some other bugs, wrong assembling and some crashes, and since you did not interact with this I had to move on but I'm planning to a project about PPC in the near future and I intend to use Keystone for it, when then I will devote a good time to work on Keystone,
and I will enhance this PR since I know about the temporary memory leak and the wrong calculation issue can come back if you deal with large code,

@aquynh
Copy link
Member

aquynh commented Nov 30, 2017

trying just one instruction like below, with "kstool"

$ kstool x32 "jne  0x1011b930" 1011b900
jne	0x1011b930 = [ 75 2e ]

confirm with Casptone's cstool:

$ cstool x32 752e 1011b900
1011b900  75 2e               jne	0x1011b930

the result matches, so i dont see anything wrong here.

can you reproduce this issue with "kstool" like above to make it easier to confirm the problem?

@TartaRikker
Copy link
Author

hummm... first sorry for being late, second I'm not able to reproduce this using kstool I even do not think that kstool is efficient on heavy testing or bugs tracking the method I talked about are much effective.

Anyways you really need to test that example with "ks_asm" API, one with this PR another without it, but in both cases you have to pass "0" in the third argument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants