Peter Beutner wrote:
Before starting to make this whole noexecute override behaviour configurable, it first must work reliable. In its current form there is no guarantee that the check_no_exec() function is actually called, because any other installed exception handler might decide to handle the exception itself. And as seen by the number of failing applications, this seems to happen quite a lot. This patch therefore makes check_no_exec() to be called before any other exception handler.
Any comments what is wrong with this one?
Peter Beutner p.beutner@gmx.net writes:
Peter Beutner wrote:
Before starting to make this whole noexecute override behaviour configurable, it first must work reliable. In its current form there is no guarantee that the check_no_exec() function is actually called, because any other installed exception handler might decide to handle the exception itself. And as seen by the number of failing applications, this seems to happen quite a lot. This patch therefore makes check_no_exec() to be called before any other exception handler.
Any comments what is wrong with this one?
If the exception is not made visible to the app then there is no point in having one at all, we might just as well turn off the protection right away. What really needs to be done is to investigate the Windows behavior and determine which parts of the app memory should be protected and which shouldn't, depending on the exe flags etc. and then replicate that behavior.
Alexandre Julliard wrote:
Peter Beutner p.beutner@gmx.net writes:
Peter Beutner wrote:
Before starting to make this whole noexecute override behaviour configurable, it first must work reliable. In its current form there is no guarantee that the check_no_exec() function is actually called, because any other installed exception handler might decide to handle the exception itself. And as seen by the number of failing applications, this seems to happen quite a lot. This patch therefore makes check_no_exec() to be called before any other exception handler.
Any comments what is wrong with this one?
If the exception is not made visible to the app then there is no point in having one at all, we might just as well turn off the protection right away.
Why should this exception be visible to the application? Plus if you make it visible, you can just forget this whole workaround idea, because it won't work reliable anyways.
And remembering the last discussion I thought "just turning off the protection" wasn't your preferred solution either.
What really needs to be done is to investigate the Windows behavior and determine which parts of the app memory should be protected and which shouldn't, depending on the exe flags etc. and then replicate that behavior.
Certainly some more testing on windows is needed, but this check_no_exec() workaround was regardless introduced a long time ago. This patch just fixes this workaround. Otherwise it could as well be removed at all.
Peter Beutner p.beutner@gmx.net writes:
Why should this exception be visible to the application? Plus if you make it visible, you can just forget this whole workaround idea, because it won't work reliable anyways.
Well, yes, the workaround is really a hack that should be replaced by a proper fix; I was hoping it would encourage someone to look into it and fix it properly, I can't do it because my box doesn't have noexec support.
Certainly some more testing on windows is needed, but this check_no_exec() workaround was regardless introduced a long time ago. This patch just fixes this workaround. Otherwise it could as well be removed at all.
Well, yes, it could certainly be removed; I added it mostly to make sure that we generated the exception properly, and to demonstrate how the exception can be handled. The proper fix is clearly more complex than that, but moving that hack into the exception code isn't a step in the right direction IMO.
Alexandre Julliard wrote:
Peter Beutner p.beutner@gmx.net writes:
Why should this exception be visible to the application? Plus if you make it visible, you can just forget this whole workaround idea, because it won't work reliable anyways.
Well, yes, the workaround is really a hack that should be replaced by a proper fix; I was hoping it would encourage someone to look into it and fix it properly, I can't do it because my box doesn't have noexec support.
Certainly some more testing on windows is needed, but this check_no_exec() workaround was regardless introduced a long time ago. This patch just fixes this workaround. Otherwise it could as well be removed at all.
Well, yes, it could certainly be removed; I added it mostly to make sure that we generated the exception properly, and to demonstrate how the exception can be handled. The proper fix is clearly more complex than that, but moving that hack into the exception code isn't a step in the right direction IMO.
hm, at least for that message box that was talked about, I don't see another choice than to put it into the exception code. But that probably should then only be the message box (and maybe a possibility to change the noexecute configuration for this app) and no code for trying to workaround the protection.
I also took a closer look on the windows behaviour. There is no (at least application visible) difference in how the memory areas(heap, stack,valloc,image) are mapped between NX disabled/enabled mode. (Apart from the mysterious fact that a bunch of more dlls are loaded when it is disabled.) The only difference is whether code runs from non-executable memory or not. I'll attach the output of a small testcase I've written which shows the memory layout for both cases.
There is however one exception. If the noexecute policy is set to OptOut[1] and windows loads a PE file where the AddressEntryPoint is in a non-executable section, NX protection is automatically switched off for this process. According to some paper[2] there are even more specific compatibility checks, e.g. for the safedisc driver. Sadly MS just speaks of "System compatibility fixes" without further detailed explanation :/
But as linux can't just switch on/off the protection for specific processes, wine has to emulate it by marking all readable memory as executable as well. And as all this happens behind the application's back, I would still go with my first proposal to just pair every PROT_READ with a PROT_EXEC in dlls/ntdll/virtual.c:VIRTUAL_GetUnixProt().
Does that sound acceptable?
1) means: apply protection for all apps, but allow to disable it for specific apps 2) http://www.uninformed.org/?v=2&a=4
run tests: execute on stack: failed execute on .rodata: failed execute on .data: failed execute on heap: failed execute on valloc: failed ----------------------------------------------- dump memory info: 0x00010000 - 0x00011fff -rw- private 0x00020000 - 0x00020fff -rw- private 0x0022d000 - 0x0022dfff grw- private 0x0022e000 - 0x0022ffff -rw- private (stack) 0x00230000 - 0x00232fff -r-- mapped 0x00240000 - 0x00243fff -rw- private (heap) 0x00340000 - 0x00345fff -rw- private 0x00350000 - 0x00352fff -rw- mapped 0x00360000 - 0x00375fff -r-- mapped 0x00380000 - 0x003bcfff -r-- mapped 0x003c0000 - 0x003c5fff -r-- mapped 0x003d0000 - 0x003d5fff -rw- private 0x003e0000 - 0x003e2fff -r-- mapped 0x003f0000 - 0x003f0fff -rwe private 0x00400000 - 0x00400fff -r-- image file: test_mem.exe (400000 - 408000) 0x00401000 - 0x00403fff -r-e image section: .text flags: r-x CODE 0x00404000 - 0x00404fff -rw- image section: .data flags: rw- DATA 0x00405000 - 0x00405fff -r-- image section: .rdata flags: r-- DATA 0x00406000 - 0x00407fff -rw- image section: .bss flags: rw- BSS 0x00410000 - 0x00450fff -r-- mapped 0x00460000 - 0x00460fff -rw- private (valloc) 0x77be0000 - 0x77be0fff -r-- image file: msvcrt.dll (77be0000 - 77c38000) 0x77be1000 - 0x77c2cfff -r-e image section: .text flags: r-x CODE 0x77c2d000 - 0x77c2efff -rw- image section: .data flags: rw- DATA 0x77c2f000 - 0x77c2ffff -rw- image section: .data flags: rw- DATA 0x77c30000 - 0x77c30fff -rw- image section: .data flags: rw- DATA 0x77c31000 - 0x77c33fff -rw- image section: .data flags: rw- DATA 0x77c34000 - 0x77c37fff -r-- image section: .rsrc flags: r-- DATA 0x7c800000 - 0x7c800fff -r-- image file: kernel32.dll (7c800000 - 7c906000) 0x7c801000 - 0x7c882fff -r-e image section: .text flags: r-x CODE 0x7c883000 - 0x7c885fff -rw- image section: .data flags: rw- DATA 0x7c886000 - 0x7c887fff -rw- image section: .data flags: rw- DATA 0x7c888000 - 0x7c905fff -r-- image section: .rsrc flags: r-- DATA 0x7c910000 - 0x7c910fff -r-- image file: ntdll.dll (7c910000 - 7c9c7000) 0x7c911000 - 0x7c98bfff -r-e image section: .text flags: r-x CODE 0x7c98c000 - 0x7c98efff -rw- image section: .data flags: rw- DATA 0x7c98f000 - 0x7c990fff -rw- image section: .data flags: rw- DATA 0x7c991000 - 0x7c9c6fff -r-- image section: .rsrc flags: r-- DATA 0x7f6f0000 - 0x7f6f6fff -r-e mapped 0x7ffb0000 - 0x7ffd3fff -r-- mapped 0x7ffdc000 - 0x7ffdcfff -rw- private 0x7ffdf000 - 0x7ffdffff -rw- private 0x7ffe0000 - 0x7ffe0fff -r-- private
run tests: execute on stack: succeeded execute on .rodata: succeeded execute on .data: succeeded execute on heap: succeeded execute on valloc: succeeded ----------------------------------------------- dump memory info: 0x00010000 - 0x00011fff -rw- private 0x00020000 - 0x00020fff -rw- private 0x00030000 - 0x00036fff -rw- private 0x0023c000 - 0x0023cfff grw- private 0x0023d000 - 0x0023ffff -rw- private (stack) 0x00240000 - 0x00242fff -r-- mapped 0x00250000 - 0x00259fff -rw- private (heap) 0x00350000 - 0x00355fff -rw- private 0x00360000 - 0x00362fff -rw- mapped 0x00370000 - 0x00385fff -r-- mapped 0x00390000 - 0x003ccfff -r-- mapped 0x003d0000 - 0x003d5fff -r-- mapped 0x003e0000 - 0x003e7fff -rw- private 0x003f0000 - 0x003f3fff -rw- private 0x00400000 - 0x00400fff -r-- image file: test_mem.exe (400000 - 408000) 0x00401000 - 0x00403fff -r-e image section: .text flags: r-x CODE 0x00404000 - 0x00404fff -rw- image section: .data flags: rw- DATA 0x00405000 - 0x00405fff -r-- image section: .rdata flags: r-- DATA 0x00406000 - 0x00407fff -rw- image section: .bss flags: rw- BSS 0x00410000 - 0x00450fff -r-- mapped 0x00460000 - 0x00461fff -r-e mapped 0x00520000 - 0x00521fff -r-e mapped 0x00530000 - 0x00530fff -rw- private 0x00540000 - 0x00540fff -rw- private 0x00550000 - 0x00551fff -r-- mapped 0x00560000 - 0x00563fff -rw- private 0x00570000 - 0x00571fff -r-- mapped 0x00580000 - 0x00580fff -rwe private 0x00590000 - 0x00592fff -r-- mapped 0x005a0000 - 0x005a2fff -rw- private 0x005e0000 - 0x006e2fff -r-- mapped 0x006f0000 - 0x0073cfff -r-e mapped 0x009f0000 - 0x009f0fff -rw- private 0x00a70000 - 0x00a70fff -rw- private (valloc) 0x5b0f0000 - 0x5b0f0fff -r-- image file: UxTheme.dll (5b0f0000 - 5b128000) 0x5b0f1000 - 0x5b120fff -r-e image section: .text flags: r-x CODE 0x5b121000 - 0x5b121fff -rw- image section: .data flags: rw- DATA 0x5b122000 - 0x5b127fff -r-- image section: .rsrc flags: r-- DATA 0x5cf00000 - 0x5cf00fff -r-- image file: ShimEng.dll (5cf00000 - 5cf26000) 0x5cf01000 - 0x5cf0efff -r-e image section: .text flags: r-x CODE 0x5cf0f000 - 0x5cf11fff -rw- image section: .data flags: rw- DATA 0x5cf12000 - 0x5cf21fff -rw- image section: .data flags: rw- DATA 0x5cf22000 - 0x5cf22fff -rw- image section: .data flags: rw- DATA 0x5cf23000 - 0x5cf25fff -r-- image section: .rsrc flags: r-- DATA 0x5d450000 - 0x5d450fff -r-- image file: comctl32.dll (5d450000 - 5d4e7000) 0x5d451000 - 0x5d4c0fff -r-e image section: .text flags: r-x CODE 0x5d4c1000 - 0x5d4c2fff -rw- image section: .data flags: rw- DATA 0x5d4c3000 - 0x5d4c3fff -rw- image section: .data flags: rw- DATA 0x5d4c4000 - 0x5d4e6fff -r-- image section: .rsrc flags: r-- DATA 0x6fd90000 - 0x6fd90fff -r-- image file: AcGenral.DLL (6fd90000 - 6ff5a000) 0x6fd91000 - 0x6fdc2fff -r-e image section: .text flags: r-x CODE 0x6fdc3000 - 0x6fdc3fff -rw- image section: .data flags: rw- DATA 0x6fdc4000 - 0x6fdc7fff -rw- image section: .data flags: rw- DATA 0x6fdc8000 - 0x6fdc8fff -rw- image section: .data flags: rw- DATA 0x6fdc9000 - 0x6fdc9fff -rw- image section: .data flags: rw- DATA 0x6fdca000 - 0x6fdcbfff -rw- image section: .data flags: rw- DATA 0x6fdcc000 - 0x6ff59fff -r-- image section: .rsrc flags: r-- DATA 0x76620000 - 0x76620fff -r-- image file: USERENV.dll (76620000 - 766d5000) 0x76621000 - 0x766bffff -r-e image section: .text flags: r-x CODE 0x766c0000 - 0x766c1fff -rw- image section: .data flags: rw- DATA 0x766c2000 - 0x766d4fff -r-- image section: .rsrc flags: r-- DATA 0x76af0000 - 0x76af0fff -r-- image file: WINMM.dll (76af0000 - 76b1e000) 0x76af1000 - 0x76b0ffff -r-e image section: .text flags: r-x CODE 0x76b10000 - 0x76b10fff -rw- image section: .data flags: rw- DATA 0x76b11000 - 0x76b11fff -rw- image section: .data flags: rw- DATA 0x76b12000 - 0x76b1dfff -r-- image section: .rsrc flags: r-- DATA 0x770f0000 - 0x770f0fff -r-- image file: OLEAUT32.dll (770f0000 - 7717c000) 0x770f1000 - 0x77171fff -r-e image section: .text flags: r-x CODE 0x77172000 - 0x77172fff -rw- image section: .data flags: rw- DATA 0x77173000 - 0x77174fff -rw- image section: .data flags: rw- DATA 0x77175000 - 0x7717bfff -r-- image section: .rsrc flags: r-- DATA 0x773a0000 - 0x773a0fff -r-- image file: comctl32.dll (773a0000 - 774a2000) 0x773a1000 - 0x77430fff -r-e image section: .text flags: r-x CODE 0x77431000 - 0x77431fff -rw- image section: .data flags: rw- DATA 0x77432000 - 0x774a1fff -r-- image section: .rsrc flags: r-- DATA 0x774b0000 - 0x774b0fff -r-- image file: ole32.dll (774b0000 - 775ed000) 0x774b1000 - 0x775d5fff -r-e image section: .text flags: r-x CODE 0x775d6000 - 0x775d6fff -rw- image section: .data flags: rw- DATA 0x775d7000 - 0x775dcfff -rw- image section: .data flags: rw- DATA 0x775dd000 - 0x775ecfff -r-- image section: .rsrc flags: r-- DATA 0x77bb0000 - 0x77bb0fff -r-- image file: MSACM32.dll (77bb0000 - 77bc5000) 0x77bb1000 - 0x77bc0fff -r-e image section: .text flags: r-x CODE 0x77bc1000 - 0x77bc1fff -rw- image section: .data flags: rw- DATA 0x77bc2000 - 0x77bc4fff -r-- image section: .rsrc flags: r-- DATA 0x77bd0000 - 0x77bd0fff -r-- image file: VERSION.dll (77bd0000 - 77bd8000) 0x77bd1000 - 0x77bd4fff -r-e image section: .text flags: r-x CODE 0x77bd5000 - 0x77bd5fff -rw- image section: .data flags: rw- DATA 0x77bd6000 - 0x77bd7fff -r-- image section: .rsrc flags: r-- DATA 0x77be0000 - 0x77be0fff -r-- image file: msvcrt.dll (77be0000 - 77c38000) 0x77be1000 - 0x77c2cfff -r-e image section: .text flags: r-x CODE 0x77c2d000 - 0x77c2efff -rw- image section: .data flags: rw- DATA 0x77c2f000 - 0x77c2ffff -rw- image section: .data flags: rw- DATA 0x77c30000 - 0x77c30fff -rw- image section: .data flags: rw- DATA 0x77c31000 - 0x77c33fff -rw- image section: .data flags: rw- DATA 0x77c34000 - 0x77c37fff -r-- image section: .rsrc flags: r-- DATA 0x77d10000 - 0x77d10fff -r-- image file: USER32.dll (77d10000 - 77da0000) 0x77d11000 - 0x77d6ffff -r-e image section: .text flags: r-x CODE 0x77d70000 - 0x77d70fff -rw- image section: .data flags: rw- DATA 0x77d71000 - 0x77d71fff -rw- image section: .data flags: rw- DATA 0x77d72000 - 0x77d9ffff -r-- image section: .rsrc flags: r-- DATA 0x77da0000 - 0x77da0fff -r-- image file: ADVAPI32.dll (77da0000 - 77e4a000) 0x77da1000 - 0x77e15fff -r-e image section: .text flags: r-x CODE 0x77e16000 - 0x77e16fff -rw- image section: .data flags: rw- DATA 0x77e17000 - 0x77e1afff -rw- image section: .data flags: rw- DATA 0x77e1b000 - 0x77e49fff -r-- image section: .rsrc flags: r-- DATA 0x77e50000 - 0x77e50fff -r-- image file: RPCRT4.dll (77e50000 - 77ee1000) 0x77e51000 - 0x77ed9fff -r-e image section: .text flags: r-x CODE 0x77eda000 - 0x77edafff -rw- image section: .data flags: rw- DATA 0x77edb000 - 0x77ee0fff -r-- image section: .rsrc flags: r-- DATA 0x77ef0000 - 0x77ef0fff -r-- image file: GDI32.dll (77ef0000 - 77f36000) 0x77ef1000 - 0x77f31fff -r-e image section: .text flags: r-x CODE 0x77f32000 - 0x77f32fff -rw- image section: .data flags: rw- DATA 0x77f33000 - 0x77f35fff -r-- image section: .rsrc flags: r-- DATA 0x77f40000 - 0x77f40fff -r-- image file: SHLWAPI.dll (77f40000 - 77fb6000) 0x77f41000 - 0x77facfff -r-e image section: .text flags: r-x CODE 0x77fad000 - 0x77fadfff -rw- image section: .data flags: rw- DATA 0x77fae000 - 0x77fb5fff -r-- image section: .rsrc flags: r-- DATA 0x7c800000 - 0x7c800fff -r-- image file: kernel32.dll (7c800000 - 7c906000) 0x7c801000 - 0x7c882fff -r-e image section: .text flags: r-x CODE 0x7c883000 - 0x7c885fff -rw- image section: .data flags: rw- DATA 0x7c886000 - 0x7c887fff -rw- image section: .data flags: rw- DATA 0x7c888000 - 0x7c905fff -r-- image section: .rsrc flags: r-- DATA 0x7c910000 - 0x7c910fff -r-- image file: ntdll.dll (7c910000 - 7c9c7000) 0x7c911000 - 0x7c98bfff -r-e image section: .text flags: r-x CODE 0x7c98c000 - 0x7c98efff -rw- image section: .data flags: rw- DATA 0x7c98f000 - 0x7c990fff -rw- image section: .data flags: rw- DATA 0x7c991000 - 0x7c9c6fff -r-- image section: .rsrc flags: r-- DATA 0x7c9d0000 - 0x7c9d0fff -r-- image file: SHELL32.dll (7c9d0000 - 7d1ee000) 0x7c9d1000 - 0x7cbcbfff -r-e image section: .text flags: r-x CODE 0x7cbcc000 - 0x7cbdbfff -rw- image section: .data flags: rw- DATA 0x7cbdc000 - 0x7cbe1fff -rw- image section: .data flags: rw- DATA 0x7cbe2000 - 0x7cbe8fff -rw- image section: .data flags: rw- DATA 0x7cbe9000 - 0x7d1edfff -r-- image section: .rsrc flags: r-- DATA 0x7f6f0000 - 0x7f6f6fff -r-e mapped 0x7ffb0000 - 0x7ffd3fff -r-- mapped 0x7ffdc000 - 0x7ffdcfff -rw- private 0x7ffdf000 - 0x7ffdffff -rw- private 0x7ffe0000 - 0x7ffe0fff -r-- private
Peter Beutner p.beutner@gmx.net writes:
But as linux can't just switch on/off the protection for specific processes, wine has to emulate it by marking all readable memory as executable as well. And as all this happens behind the application's back, I would still go with my first proposal to just pair every PROT_READ with a PROT_EXEC in dlls/ntdll/virtual.c:VIRTUAL_GetUnixProt().
Does that sound acceptable?
Given your investigations that sounds reasonable, yes, thanks for taking the time to look into this. Does something like this work for you?
diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 99c90c4..4d15779 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -2126,6 +2126,8 @@ void WINAPI LdrInitializeThunk( ULONG unknown1, ULONG unknown2, ULONG unknown3,
peb->ProcessParameters->ImagePathName = wm->ldr.FullDllName; version_init( wm->ldr.FullDllName.Buffer ); + if (!(nt->OptionalHeader.DllCharacteristics & IMAGE_DLLCHARACTERISTICS_NX_COMPAT)) + VIRTUAL_SetForceExec( TRUE );
/* the main exe needs to be the first in the load order list */ RemoveEntryList( &wm->ldr.InLoadOrderModuleList ); diff --git a/dlls/ntdll/ntdll_misc.h b/dlls/ntdll/ntdll_misc.h index 781afa5..a7ef8df 100644 --- a/dlls/ntdll/ntdll_misc.h +++ b/dlls/ntdll/ntdll_misc.h @@ -112,6 +112,7 @@ extern NTSTATUS DIR_get_unix_cwd( char **cwd ); /* virtual memory */ extern NTSTATUS VIRTUAL_HandleFault(LPCVOID addr); extern BOOL VIRTUAL_HasMapping( LPCVOID addr ); +extern void VIRTUAL_SetForceExec( BOOL enable ); extern void VIRTUAL_UseLargeAddressSpace(void);
extern BOOL is_current_process( HANDLE handle ); diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c index 573072e..5da929b 100644 --- a/dlls/ntdll/virtual.c +++ b/dlls/ntdll/virtual.c @@ -143,6 +143,9 @@ static void *user_space_limit = USER_SPACE_LIMIT; static void *preload_reserve_start; static void *preload_reserve_end; static int use_locks; +static int force_exec_prot; /* whether to force PROT_EXEC on all PROT_READ mmaps */ + +static int VIRTUAL_GetUnixProt( BYTE vprot );
/*********************************************************************** @@ -391,6 +394,7 @@ static NTSTATUS create_view( struct file_view **view_ret, void *base, size_t siz { struct file_view *view; struct list *ptr; + int unix_prot = VIRTUAL_GetUnixProt( vprot );
assert( !((UINT_PTR)base & page_mask) ); assert( !(size & page_mask) ); @@ -446,6 +450,12 @@ static NTSTATUS create_view( struct file_view **view_ret, void *base, size_t siz
*view_ret = view; VIRTUAL_DEBUG_DUMP_VIEW( view ); + + if (force_exec_prot && (unix_prot & PROT_READ) && !(unix_prot & PROT_EXEC)) + { + TRACE( "forcing exec permission on %p-%p\n", base, (char *)base + size - 1 ); + mprotect( base, size, unix_prot | PROT_EXEC ); + } return STATUS_SUCCESS; }
@@ -555,12 +565,22 @@ static BOOL VIRTUAL_SetProt( FILE_VIEW *view, /* [in] Pointer to view */ size_t size, /* [in] Size in bytes */ BYTE vprot ) /* [in] Protections to use */ { + int unix_prot = VIRTUAL_GetUnixProt(vprot); + TRACE("%p-%p %s\n", base, (char *)base + size - 1, VIRTUAL_GetProtStr( vprot ) );
- if (mprotect( base, size, VIRTUAL_GetUnixProt(vprot) )) - return FALSE; /* FIXME: last error */ + if (force_exec_prot && (unix_prot & PROT_READ) && !(unix_prot & PROT_EXEC)) + { + TRACE( "forcing exec permission on %p-%p\n", base, (char *)base + size - 1 ); + if (!mprotect( base, size, unix_prot | PROT_EXEC )) goto done; + /* exec + write may legitimately fail, in that case fall back to write only */ + if (!(unix_prot & PROT_WRITE)) return FALSE; + }
+ if (mprotect( base, size, unix_prot )) return FALSE; /* FIXME: last error */ + +done: memset( view->prot + (((char *)base - (char *)view->base) >> page_shift), vprot, size >> page_shift ); VIRTUAL_DEBUG_DUMP_VIEW( view ); @@ -1285,6 +1305,61 @@ BOOL VIRTUAL_HasMapping( LPCVOID addr )
/*********************************************************************** + * VIRTUAL_SetForceExec + * + * Whether to force exec prot on all views. + */ +void VIRTUAL_SetForceExec( BOOL enable ) +{ + struct file_view *view; + + RtlEnterCriticalSection( &csVirtual ); + if (!force_exec_prot != !enable) /* change all existing views */ + { + force_exec_prot = enable; + + LIST_FOR_EACH_ENTRY( view, &views_list, struct file_view, entry ) + { + UINT i, count; + int unix_prot; + char *addr = view->base; + BYTE prot = view->prot[0]; + + for (count = i = 1; i < view->size >> page_shift; i++, count++) + { + if (view->prot[i] == prot) continue; + unix_prot = VIRTUAL_GetUnixProt( prot ); + if ((unix_prot & PROT_READ) && !(unix_prot & PROT_EXEC)) + { + TRACE( "%s exec prot for %p-%p\n", + force_exec_prot ? "enabling" : "disabling", + addr, addr + (count << page_shift) - 1 ); + mprotect( addr, count << page_shift, + unix_prot | (force_exec_prot ? PROT_EXEC : 0) ); + } + addr += (count << page_shift); + prot = view->prot[i]; + count = 0; + } + if (count) + { + unix_prot = VIRTUAL_GetUnixProt( prot ); + if ((unix_prot & PROT_READ) && !(unix_prot & PROT_EXEC)) + { + TRACE( "%s exec prot for %p-%p\n", + force_exec_prot ? "enabling" : "disabling", + addr, addr + (count << page_shift) - 1 ); + mprotect( addr, count << page_shift, + unix_prot | (force_exec_prot ? PROT_EXEC : 0) ); + } + } + } + } + RtlLeaveCriticalSection( &csVirtual ); +} + + +/*********************************************************************** * VIRTUAL_UseLargeAddressSpace * * Increase the address space size for apps that support it.
Alexandre Julliard wrote:
Peter Beutner p.beutner@gmx.net writes:
But as linux can't just switch on/off the protection for specific processes, wine has to emulate it by marking all readable memory as executable as well. And as all this happens behind the application's back, I would still go with my first proposal to just pair every PROT_READ with a PROT_EXEC in dlls/ntdll/virtual.c:VIRTUAL_GetUnixProt().
Does that sound acceptable?
Given your investigations that sounds reasonable, yes, thanks for taking the time to look into this. Does something like this work for you?
Yes, it works. This should be pretty much equal to the default OptIn policy under windows, i.e. disable the protection for everyone unless the app explicitely say it can handle it. (except for the fact that wine always does this now, even if the cpu hasn't NX or something aquivalent.Shouldn't really hurt anyone though.)
diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 99c90c4..4d15779 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -2126,6 +2126,8 @@ void WINAPI LdrInitializeThunk( ULONG unknown1, ULONG unknown2, ULONG unknown3,
peb->ProcessParameters->ImagePathName = wm->ldr.FullDllName; version_init( wm->ldr.FullDllName.Buffer );
- if (!(nt->OptionalHeader.DllCharacteristics & IMAGE_DLLCHARACTERISTICS_NX_COMPAT))
VIRTUAL_SetForceExec( TRUE );
I'm not so sure about this, I think I read somewhere that each loaded DLL is checked for this not just the main exe and the protection is disabled if at least on module is not nx compatible. But as no wine dll is marked as NX_COMPAT(i assume) this would basically mean to always disable the protection. It probably doesn't hurt to leave it like this for now. The rest (build wine dlls with NX_COMPAT and do this check for every lib) can still be added later. I think it's not quite common anyway that the main exe is flagged as NX_COMPAT and some dll of the app is not ;)
Peter Beutner p.beutner@gmx.net writes:
I'm not so sure about this, I think I read somewhere that each loaded DLL is checked for this not just the main exe and the protection is disabled if at least on module is not nx compatible. But as no wine dll is marked as NX_COMPAT(i assume) this would basically mean to always disable the protection. It probably doesn't hurt to leave it like this for now.
Yes, the next step is to implement a smarter strategy for turning protection on or off. I don't know about checking the flag for all dlls or only the main exe, I guess I'll have to ask you to try this on Windows too...
-
Alexandre Julliard -
Peter Beutner