[PATCH] ntdll: Improve handling of THUMB_MOV32 relocations
martin at martin.st
Thu Jan 2 14:57:28 CST 2014
On Thu, 2 Jan 2014, André Hentschel wrote:
> Am 02.01.2014 17:01, schrieb Martin Storsjo:
>> Since the delta variable actually is signed, the delta > 0xffff
>> condition could be false even if the high part of the delta
>> was nonzero, ending up with not updating the high half of the
>> relocation at all.
>> Additionally, carry any overflow from the low word into the
>> high word delta.
> Do you have an App that triggers that?
I've got a 5 MB .exe that triggers this occasionally. If I disable ASLR in
the kernel, it doesn't trigger. I guess the bigger the image, the larger
the risk that ASLR will force the exe to be relocated, triggering the
Conversely, if I change map_image to not try to load the image at the base
address (as if it would have failed to load at that address due to ASLR),
this triggers every time even with a small hello world .exe.
> So you mean the case when the sign bit is used, e.g. 10101010b... Then the delta is negative, but is everything set correctly in the instruction?
Yes, the calculation seem to work just fine with my patch applied. With
the 5 MB .exe, the relocation that is done on startup was:
trace:module:map_image relocating from 0x400000-0xe6e000 to 0xb58a0000-0xb630e000
So the delta really isn't negative per se, but it's too large to represent
with a 32 bit signed integer.
So to reproduce easily, apply this diff to your tree, then try to execute
http://albin.abo.fi/~mstorsjo/helloworld-arm.exe (a plain hello world exe
compiled with msvc 2012 for winrt). At least here, it tries to relocate
the .exe from 0x400000-0x410000 to 0xb6200000-0xb6210000.
index e93ce2e..a6ffe38 100644
@@ -1077,9 +1077,6 @@ static NTSTATUS map_image( HANDLE hmapping, int fd,
char *base, SIZE_T total_siz
server_enter_uninterrupted_section( &csVirtual, &sigset );
- if (base >= (char *)address_space_start) /* make sure the DOS area
remains free */
- status = map_view( &view, base, total_size, mask, FALSE,
- VPROT_COMMITTED | VPROT_READ | VPROT_EXEC |
VPROT_WRITECOPY | VPROT_IMAGE );
if (status != STATUS_SUCCESS)
status = map_view( &view, NULL, total_size, mask, FALSE,
>> dlls/ntdll/loader.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>> diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c
>> index e0907e7..1f13424 100644
>> --- a/dlls/ntdll/loader.c
>> +++ b/dlls/ntdll/loader.c
>> @@ -2230,19 +2230,19 @@ IMAGE_BASE_RELOCATION * WINAPI LdrProcessRelocationBlock( void *page, UINT count
>> DWORD inst = *(INT_PTR *)((char *)page + offset);
>> DWORD imm16 = ((inst << 1) & 0x0800) + ((inst << 12) & 0xf000) +
>> ((inst >> 20) & 0x0700) + ((inst >> 16) & 0x00ff);
>> + int hi_delta;
> Why you're using int here? DWORD looks much better for this.
No particular reason, DWORD should work just fine as well, it seems. I can
send a new patch with that changed if you're ok with the rest of it - just
let me know.
More information about the wine-devel