1

I'm learning how to hotpatch functions and I have the following code which works fine in 32-bit programs. However, I'm trying to get it to work in 64-bit programs as well but it just crashes.

#ifndef __x86_64
std::uint8_t Store[8] = {0};
#else
std::uint8_t Store[15] = {0};
#endif

void Patch(std::uint8_t* OriginalAddress, std::uint8_t* ReplacementAddress)
{
    #ifndef __x86_64
    const static std::uint8_t jmp[] = {0xb8, 0x00, 0x00, 0x00, 0x00, 0xff, 0xe0}; /** movl $0x0, %eax ;;; jmp *%eax **/
    #else
    const static std::uint8_t jmp[] = {0x48, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xe0, 0x90, 0x90}; /** movq $0x0, %rax ;;; jmp *%rax **/
    #endif

    DWORD dwProtect = 0;
    const static std::int8_t jmp_size = sizeof(jmp) / sizeof(std::uint8_t);
    VirtualProtect(OriginalAddress, jmp_size, PAGE_EXECUTE_READWRITE, &dwProtect);
    memcpy(Store, OriginalAddress, jmp_size);
    memcpy(OriginalAddress, jmp, jmp_size);
    memcpy(OriginalAddress + 1, &ReplacementAddress, sizeof(void*));
    VirtualProtect(OriginalAddress, jmp_size, dwProtect, &dwProtect);
}

Any ideas what is wrong with the code?

Brandon
  • 22,723
  • 11
  • 93
  • 186
  • Have you tried stepping through the code instruction-by-instruction in your debugger's disassembly view? Also you may want to provide a complete minimal example with the code to be patched. – doynax Dec 19 '13 at 18:23
  • Yes sir. That is how I got the hex values for the instructions. I also enabled -masm=intel to make it easier to read dumps. No luck still :l – Brandon Dec 19 '13 at 18:26
  • 2
    "it just crashes" is just one step up from "it doesn't work", it's still far too little detail, especially if you want to deal with code at the machine level. It's your code. You should be familiar with it. Where does it crash? In what way does it crash? –  Dec 19 '13 at 18:26
  • Which instruction leads to the crash? MOV or JMP? – Hristo Iliev Dec 19 '13 at 18:34
  • Quick note on best practices... This `sizeof(jmp) / sizeof(std::uint8_t)` is clearer, more maintainable, and easier to immediately understand when written as `sizeof(jmp) / sizeof(jmp[0])`, since that's what you want, anyway. – Parthian Shot Mar 03 '16 at 16:48

1 Answers1

3
const static std::uint8_t jmp[] = {0x48, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xe0, 0x90, 0x90};

...

memcpy(OriginalAddress + 1, &ReplacementAddress, sizeof(void*));

For your x86-64 instructions, the address to jump to, the 0x0000000000000000 in your jmp array, starts at the third byte, not at the second. You're overwriting part of the mov instruction, and what you end up with is an invalid instruction if you're lucky.

As a side note, I'm doubtful it's safe to just overwrite %eax/%rax like that. Have you determined that those registers will never contain any values of interest?

Community
  • 1
  • 1
  • If this is the start of a function, it's safe to overwrite EAX/RAX, since that register is volatile (neither caller-saved nor callee-saved) for function calls. – Adam Rosenfield Dec 19 '13 at 18:45
  • This solved it. I had to change the memcpy like you said. That works like a charm. Yes I am sure eax and rax has nothing of interest. – Brandon Dec 19 '13 at 18:51
  • @AdamRosenfield It depends on the calling convention. At least one calling convention that I regularly use does use EAX for some function arguments. But if the OP is certain that no such function will be hotpatched, it should be okay. –  Dec 19 '13 at 18:59