Menu
▾
▴
#537 pseudo_reloc fails to mark pages writable
(My original report of this issue can be found here.)
It appears that the pseudo relocation code sometimes tries to write an immediate value to code in a read-only memory region (part of the .text section) due to a critical bug. This causes an affected application to 'Segmentation fault' at start up.
This has been observed for a large number of applications in a clean install of the MSYS2 based Git for Windows SDK:
- /mingw32/bin/gdbm_load.exe
- /mingw32/bin/git.exe
- /mingw32/bin/git-shell.exe
- /mingw32/bin/git-upload-pack.exe
- /mingw32/bin/gnutls-cli.exe
- /mingw32/bin/gnutls-serv.exe
- /mingw32/bin/nettle-hash.exe
- /mingw32/bin/ocsptool.exe
- /mingw32/bin/pcregrep.exe
- /mingw32/bin/rtmpdump.exe
- /mingw32/bin/rtmpgw.exe
- /mingw32/bin/rtmpsrv.exe
- /mingw32/bin/rtmpsuck.exe
- /mingw32/bin/sexp-conv.exe
- /mingw32/bin/WhoUses.exe
So it clearly has high chance of occurrence (but may be affected by environment in some way). The problem has been seen on Windows XP, Windows 7 and reportedly Windows 10 also. It can be expected that some interfering processes on the system such as antivirus/malware/debuggers would make this problem more likely to occur.
Essentially, the code for making the memory writable is completely wrong, and has zero chance of working on any system where any memory pages in the .text section may have had their attributes changed since the application was loaded. This is because VirtualQuery only provides information for a contiguous region of pages which have the same attributes. This means RegionSize can be pretty much anything (it could be as small as 0x1000 for a single page). Also it only gives information starting at the page containing the address specified (so just giving it the address you want to write to is not good for this use case, where a lot of pages need to be written over).
Since it's impossible to know the base address of the page range, which shares the same attributes as the actual location you want to write to, the only option I can think of is to scan through the whole section until you find the right one. Once found, you need to keep hold of it (in sec_start, which should be renamed to region_start) and you also need to keep the RegionSize (that would be added as a new field region_size in that structure). The range check then becomes (region_start <= addr) && (addr < region_start + region_size). Once done with the region you just need to call VirtualProtect again with the old attributes (so doing another VirtualQuery, as it currently does in restore_modified_sections, is totally pointless).
Obviously the naming of these functions and data structures is invalid, since for operating on the entire section to work, you'd need to change the attributes for every page in the section (which would make it impossible to restore the original attributes). Also the way it allocates the memory for the sections is unsuitable since the number of page ranges may be much larger than section count.
Here's my manually annotated x86 ASM of the problem code in a compiled program (git.exe) for reference:
0056B2B0 > $ 57 PUSH EDI ; entry: mark_section_writable
0056B2B1 . 56 PUSH ESI ; param1: EAX(addr)
0056B2B2 . 53 PUSH EBX
0056B2B3 . 83EC 30 SUB ESP,30 ; stack: <params>, b(ESP+0x14)
0056B2B6 . 8B35 40E26000 MOV ESI,DWORD PTR DS:[60E240] ; ESI = maxSections
0056B2BC . 85F6 TEST ESI,ESI
0056B2BE . 0F8E D9000000 JLE git.0056B39D
0056B2C4 . 8B3D 44E26000 MOV EDI,DWORD PTR DS:[60E244] ; EDI = the_secs
0056B2CA . 31DB XOR EBX,EBX ; EBX(i) = 0
0056B2CC . 8D57 04 LEA EDX,DWORD PTR DS:[EDI+4] ; EDX = &the_secs[0].sec_start
0056B2CF . 90 NOP
0056B2D0 > 8B0A MOV ECX,DWORD PTR DS:[EDX] ; ECX = the_secs[i].sec_start
0056B2D2 . 39C1 CMP ECX,EAX
0056B2D4 . 77 0E JA SHORT git.0056B2E4 ; jump if the_secs[i].sec_start > addr
0056B2D6 . 8B7A 04 MOV EDI,DWORD PTR DS:[EDX+4] ; EDI = the_secs[i].hash
0056B2D9 . 034F 08 ADD ECX,DWORD PTR DS:[EDI+8] ; ECX += the_secs[i].hash->Misc.VirtualSize
0056B2DC . 39C8 CMP EAX,ECX
0056B2DE . 0F82 B2000000 JB git.0056B396 ; jump if addr < ECX
0056B2E4 > 83C3 01 ADD EBX,1 ; i += 1
0056B2E7 . 83C2 0C ADD EDX,0C ; EDX = &the_secs[i]
0056B2EA . 39F3 CMP EBX,ESI
0056B2EC .^ 75 E2 JNZ SHORT git.0056B2D0 ; jump if EBX(i) != ESI(maxSections)
0056B2EE > 890424 MOV DWORD PTR SS:[ESP],EAX ; |<param1> = addr
0056B2F1 . 89C3 MOV EBX,EAX ; |
0056B2F3 . E8 D8080000 CALL <git.__mingw_GetSectionForAddress> ; \__mingw_GetSectionForAddress
0056B2F8 . 85C0 TEST EAX,EAX ; EAX(h) = <section header>
0056B2FA . 89C7 MOV EDI,EAX ; EDI(h) = <section header>
0056B2FC . 0F84 D8000000 JE git.0056B3DA ; <not taken>
0056B302 . 8D1C76 LEA EBX,DWORD PTR DS:[ESI+ESI*2] ; EBX = 3*i
0056B305 . 8B35 44E26000 MOV ESI,DWORD PTR DS:[60E244] ; ESI = the_secs
0056B30B . C1E3 02 SHL EBX,2 ; EBX = 12*i
0056B30E . 01DE ADD ESI,EBX ; ESI = &the_secs[i]
0056B310 . 8946 08 MOV DWORD PTR DS:[ESI+8],EAX ; the_secs[i].hash = h
0056B313 . C706 00000000 MOV DWORD PTR DS:[ESI],0 ; the_secs[i].old_protect = 0
0056B319 . E8 E2090000 CALL <git._GetPEImageBase> ; EAX = <image base>
0056B31E . 0347 0C ADD EAX,DWORD PTR DS:[EDI+C] ; EAX += h->VirtualAddress
0056B321 . 8946 04 MOV DWORD PTR DS:[ESI+4],EAX ; the_secs[i].sec_start = EAX
0056B324 . 8D4424 14 LEA EAX,DWORD PTR SS:[ESP+14] ; EAX = &b
0056B328 . C74424 08 1C0>MOV DWORD PTR SS:[ESP+8],1C ; |<param3>(BufSize) = sizeof(b) = 28
0056B330 . 894424 04 MOV DWORD PTR SS:[ESP+4],EAX ; |<param2>(Buffer) = &b
0056B334 . A1 44E26000 MOV EAX,DWORD PTR DS:[60E244] ; |EAX = the_secs
0056B339 . 8B4418 04 MOV EAX,DWORD PTR DS:[EAX+EBX+4] ; |EAX = the_secs[i].sec_start
0056B33D . 890424 MOV DWORD PTR SS:[ESP],EAX ; |<param1>(Address) = the_secs[i].sec_start
0056B340 . FF15 24076200 CALL DWORD PTR DS:[<&KERNEL32.VirtualQue>; \VirtualQuery
0056B346 . 83EC 0C SUB ESP,0C ; (restore ESP)
0056B349 . 85C0 TEST EAX,EAX
0056B34B . 74 6D JE SHORT git.0056B3BA ; <not taken>
0056B34D . 8B4424 28 MOV EAX,DWORD PTR SS:[ESP+28] ; EAX = b.Protect
0056B351 . 8D50 FC LEA EDX,DWORD PTR DS:[EAX-4] ; if b.Protect == 0x04(PAGE_READWRITE)
0056B354 . 83E2 FB AND EDX,FFFFFFFB ; or b.Protect == 0x08(PAGE_WRITECOPY)
0056B357 . 74 36 JE SHORT git.0056B38F ; then jump
0056B359 . 83E8 40 SUB EAX,40 ; if b.Protect == 0x40(PAGE_EXECUTE_READWRITE)
0056B35C . 83E0 BF AND EAX,FFFFFFBF ; or b.Protect == 0x80(PAGE_EXECUTE_WRITECOPY)
0056B35F . 74 2E JE SHORT git.0056B38F ; then jump
0056B361 . 8B4424 20 MOV EAX,DWORD PTR SS:[ESP+20] ; EAX = b.RegionSize
0056B365 . 031D 44E26000 ADD EBX,DWORD PTR DS:[60E244] ; EBX = &the_secs[i]
0056B36B . C74424 08 400>MOV DWORD PTR SS:[ESP+8],40 ; |<param3>(NewProtect) = 0x40(PAGE_EXECUTE_READWRITE)
0056B373 . 894424 04 MOV DWORD PTR SS:[ESP+4],EAX ; |<param2>(Size) = b.RegionSize
0056B377 . 8B4424 14 MOV EAX,DWORD PTR SS:[ESP+14] ; |EAX = b.BaseAddress
0056B37B . 895C24 0C MOV DWORD PTR SS:[ESP+C],EBX ; |<param4>(pOldProtect) = &the_secs[i].old_protect
0056B37F . 890424 MOV DWORD PTR SS:[ESP],EAX ; |<param1>(Address) = b.BaseAddress
0056B382 . FF15 20076200 CALL DWORD PTR DS:[<&KERNEL32.VirtualPro>; \VirtualProtect
0056B388 . 83EC 10 SUB ESP,10 ; (restore ESP)
0056B38B . 85C0 TEST EAX,EAX
0056B38D . 74 15 JE SHORT git.0056B3A4 ; <not taken>
0056B38F > 8305 40E26000>ADD DWORD PTR DS:[60E240],1 ; maxSections += 1
0056B396 > 83C4 30 ADD ESP,30 ; (stack unwind)
0056B399 . 5B POP EBX
0056B39A . 5E POP ESI
0056B39B . 5F POP EDI
0056B39C . C3 RETN
0056B39D > 31F6 XOR ESI,ESI ; ESI(i) = 0
0056B39F .^ E9 4AFFFFFF JMP git.0056B2EE
0056B3A4 > FF15 6C066200 CALL DWORD PTR DS:[<&KERNEL32.GetLastErr>; [GetLastError
0056B3AA . C70424 64735B>MOV DWORD PTR SS:[ESP],git.005B7364 ; ASCII " VirtualProtect failed with code 0x%x"
0056B3B1 . 894424 04 MOV DWORD PTR SS:[ESP+4],EAX
0056B3B5 . E8 96FEFFFF CALL git.0056B250
0056B3BA > A1 44E26000 MOV EAX,DWORD PTR DS:[60E244]
0056B3BF . 8B4418 04 MOV EAX,DWORD PTR DS:[EAX+EBX+4]
0056B3C3 . 894424 08 MOV DWORD PTR SS:[ESP+8],EAX
0056B3C7 . 8B47 08 MOV EAX,DWORD PTR DS:[EDI+8]
0056B3CA . C70424 30735B>MOV DWORD PTR SS:[ESP],git.005B7330 ; ASCII " VirtualQuery failed for %d bytes at address %p"
0056B3D1 . 894424 04 MOV DWORD PTR SS:[ESP+4],EAX
0056B3D5 . E8 76FEFFFF CALL git.0056B250
0056B3DA > 895C24 04 MOV DWORD PTR SS:[ESP+4],EBX
0056B3DE . C70424 10735B>MOV DWORD PTR SS:[ESP],git.005B7310 ; ASCII "Address %p has no image-section"
0056B3E5 . E8 66FEFFFF CALL git.0056B250
...
0056B656 . 89C7 MOV EDI,EAX ; libssp-0.__stack_chk_guard
0056B658 . 8945 CC MOV DWORD PTR SS:[EBP-34],EAX
0056B65B . 89F0 MOV EAX,ESI ; git.0056D502
0056B65D . E8 4EFCFFFF CALL <git.mark_section_writable> ; (fails to do anything)
0056B662 . 893E MOV DWORD PTR DS:[ESI],EDI ; ***!!CRASH!!***
And the code it was apparently compiled from in mingw-w64-crt/crt/pseudo-reloc.c (sourceforge.net) (no discrepancies found):
extern int __mingw_GetSectionCount (void);
extern PIMAGE_SECTION_HEADER __mingw_GetSectionForAddress (LPVOID p);
extern PBYTE _GetPEImageBase (void);
typedef struct sSecInfo {
/* Keeps altered section flags, or zero if nothing was changed. */
DWORD old_protect;
PBYTE sec_start;
PIMAGE_SECTION_HEADER hash;
} sSecInfo;
static sSecInfo *the_secs = NULL;
static int maxSections = 0;
static void
mark_section_writable (LPVOID addr)
{
MEMORY_BASIC_INFORMATION b;
PIMAGE_SECTION_HEADER h;
int i;
for (i = 0; i < maxSections; i++)
{
if (the_secs[i].sec_start <= ((LPBYTE) addr)
&& ((LPBYTE) addr) < (the_secs[i].sec_start + the_secs[i].hash->Misc.VirtualSize))
return;
}
h = __mingw_GetSectionForAddress (addr);
if (!h)
{
__report_error ("Address %p has no image-section", addr);
return;
}
the_secs[i].hash = h;
the_secs[i].old_protect = 0;
the_secs[i].sec_start = _GetPEImageBase () + h->VirtualAddress;
if (!VirtualQuery (the_secs[i].sec_start, &b, sizeof(b)))
{
__report_error (" VirtualQuery failed for %d bytes at address %p",
(int) h->Misc.VirtualSize, the_secs[i].sec_start);
return;
}
if (b.Protect != PAGE_EXECUTE_READWRITE && b.Protect != PAGE_READWRITE
&& b.Protect != PAGE_EXECUTE_WRITECOPY && b.Protect != PAGE_WRITECOPY)
{
if (!VirtualProtect (b.BaseAddress, b.RegionSize,
PAGE_EXECUTE_READWRITE,
&the_secs[i].old_protect))
__report_error (" VirtualProtect failed with code 0x%x",
(int) GetLastError ());
}
++maxSections;
return;
}
Making the .text section writable appears to be a useful workaround for this problem on a user installation, which can be carried out using the following steps.
- Download and install Stud_PE from CGSoftLabs
- Start the application, go to File->"Open PE File" and open your mingw32/bin/git.exe
- Select the 'Sections' tab and double click the '.text' entry
- Click the check box for MEM_WRITE characteristics flag to enable it and click Save
After doing this the binaries appear to function as normal.
I am going to try to implement the necessary changes to the pseudo relocations module on my fork, and then submit a merge request.
Once this has been done, all software packages/tools need to be rebuilt and updated, so that this problem no longer occurs.
