`reset_hook()` fails in interpose library when `caller` calls hooked function
steven-michaud opened this issue · 1 comments
Here's an example patch hook from the hook library template:
int (*patch_example_caller)(char *arg1, int (*arg2)(char *)) = NULL;
static int Hooked_patch_example(char *arg1, int (*arg2)(char *))
{
int retval = patch_example_caller(arg1, arg2);
LogWithFormat(true, "Hook.mm: patch_example(): arg1 \"%s\", arg2 \'%p\', returning \'%i\'",
arg1, arg2, retval);
PrintStackTrace();
// Example of using add_patch_hook() to dynamically add a patch hook for
// 'arg2'.
add_patch_hook(reinterpret_cast<void*>(arg2),
reinterpret_cast<void*>(dynamic_patch_example));
// Not always required, but using it when not required does no harm.
reset_hook(reinterpret_cast<void*>(Hooked_patch_example));
return retval;
}
The bug is that, whenever patch_example_caller()
indirectly calls a function that has also been hooked with a patch hook, reset_hook()
doesn't work -- it doesn't re-hook patch_example_caller()
.
Most of the time this doesn't matter. Most functions have a standard prologue in machine code:
push %rbp
mov %rsp, %rbp
For patch hooks on these functions, the call to reset_hook()
is in effect a no-op, and isn't necessary. But for functions without a standard prologue, not calling reset_hook()
at the end of your patch hook means that the hook is only called once. The same applies if the call to reset_hook()
fails.
The bug is caused by a problem with the code I added (in HookCase versions 3.3 and 3.3.1) to support dynamic patch hooks. I assumed that while a patch hook is running, no other patch hook can run on the same thread. But this isn't true -- other patch hooks will run on the current thread if they're called (indirectly) from the hook's "caller" (which calls the hook's original function).
This only makes a difference if you patch a function with a non-standard prologue. This is something I don't do very often, so it took me a while to notice the bug.
I have a fix for this bug, which I'm about to land.