[PATCH 2/3] kernelbase: Fix hotpatch prolog in GetProcAddress() on x64.

[Paul Gofman]

On 10/27/20 22:37, Alexandre Julliard wrote:
> Paul Gofman <pgofman at codeweavers.com> writes:
>
>> Signed-off-by: Paul Gofman <pgofman at codeweavers.com>
>> ---
>>  dlls/kernelbase/loader.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/dlls/kernelbase/loader.c b/dlls/kernelbase/loader.c
>> index 4b615e4a08f..4fb4293ca2b 100644
>> --- a/dlls/kernelbase/loader.c
>> +++ b/dlls/kernelbase/loader.c
>> @@ -439,7 +439,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetModuleHandleExW( DWORD flags, LPCWSTR name, HMO
>>   * This wrapper saves xmm0 - 3 to the stack.
>>   */
>>  __ASM_GLOBAL_FUNC( GetProcAddress,
>> -                   ".byte 0x48\n\t"  /* hotpatch prolog */
>> +                   ".byte 0x48,0x8d,0xa4,0x24,0x00,0x00,0x00,0x00\n\t"  /* hotpatch prolog */
> Is there anything that depends on such a prolog? And is there any
> evidence that Windows is using this?  AFAIU making the first instruction
> two bytes should be sufficient.
>
I don't think I ever noticed anything requiring hotpatch prologue here,
nor I saw any evidence if Windows has hotpatch prologue here or not. I
guess a dangling REX prefix can confuse some patchers though, at least
that was the case when our import thunks had such. From what I saw
debugging some hotpatchers on x64 they are overall less demanding
compared to i386 for non-syscall thunk functions and try to guess the
first opcode size (while not necessarily understand any possible opcodes
and may mess up).

So I guess we could just remove the ".byte 0x48" instead of putting the
"official" hotpatch prologue as well, unless we get any evidence that
something else is needed.