Mike McCormack schrieb:
This should fix bug #6622.
The application in question doesn't crash because it needs an executable stack, but because the PE image header of the exe is totally broken, no section is marked as executable there at all. Wine does exactly what it should do here(same goes for Irfanview in #6129, seems both apps use the same exe packer(ASPack) so probably it's the packer which is broken). I bet the applications would also crash under windows if the noexecute protection is activated.
The reason why -z execstack helps is that the linux kernel marks _all_ readable pages as executable if an executable stack is requested, not only the stack(for whatever reasons), i.e. basically switching off the whole NX protection thing. So for fixing some _broken_ applications this patch unconditionally disables nx protection for every application running under wine. Seems like a bad tradeoff imo. (Though I don't know how widespread these kind of broken applications are. But there are definitly applications out there which don't need this.)
I tried to solve the same problem last weekend in a different way, see attached patch. It's not quite ready yet though.
It basically adds an option(HKCU\Software\Wine\NoExecCompat=On|Off) which, if enabled, couples PROT_READ with PROT_EXEC in every mprotect/mmap call, i.e. emulating the old standard x86 behaviour.
(The main problem is that this config option should be read out as early as possible from the registry but on the other hand a number of things need to be set up before it is possible to access the registry. So I'm not sure when the earliest/best point would be to do this. For now it is done right before the main exe is loaded.)
This way would have the advantage that the behaviour is tunable either globally or on a per-app basis.
dlls/ntdll/loader.c | 2 + dlls/ntdll/loadorder.c | 78 +++++++++++++++++++++++++--------------------- dlls/ntdll/ntdll_misc.h | 6 +++ dlls/ntdll/virtual.c | 57 +++++++++++++++++++++++++++++++++- 4 files changed, 106 insertions(+), 37 deletions(-)
diff --git a/dlls/kernel32/process.c b/dlls/kernel32/process.c diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 87767e5..64ed609 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -1728,6 +1728,8 @@ static NTSTATUS load_dll( LPCWSTR load_p main_exe = get_modref( NtCurrentTeb()->Peb->ImageBaseAddress ); loadorder = get_load_order( main_exe ? main_exe->ldr.BaseDllName.Buffer : NULL, filename );
+ if(!main_exe) init_noexec_setting(filename); + switch(loadorder) { case LO_INVALID: diff --git a/dlls/ntdll/loadorder.c b/dlls/ntdll/loadorder.c index c31b633..552ce87 100644 --- a/dlls/ntdll/loadorder.c +++ b/dlls/ntdll/loadorder.c @@ -294,31 +294,35 @@ static inline enum loadorder get_env_loa * * Return a handle to the standard DllOverrides registry section. */ -static HANDLE get_standard_key(void) +HANDLE get_standard_key( const WCHAR *subkey ) { - static const WCHAR DllOverridesW[] = {'S','o','f','t','w','a','r','e','\','W','i','n','e','\', - 'D','l','l','O','v','e','r','r','i','d','e','s',0}; - static HANDLE std_key = (HANDLE)-1; + static const WCHAR winekeyW[] = {'S','o','f','t','w','a','r','e','\','W','i','n','e',0}; + HANDLE std_key, root; + OBJECT_ATTRIBUTES attr; + UNICODE_STRING nameW; + WCHAR *str = NULL; + + if(subkey) { + str = RtlAllocateHeap( GetProcessHeap(), HEAP_ZERO_MEMORY, + sizeof(winekeyW) + strlenW(subkey) * sizeof(WCHAR)); + if(!str) return 0; + strcatW(str, winekeyW); + strcatW(str, subkey); + RtlInitUnicodeString( &nameW, str ); + } else + RtlInitUnicodeString( &nameW, winekeyW );
- if (std_key == (HANDLE)-1) - { - OBJECT_ATTRIBUTES attr; - UNICODE_STRING nameW; - HANDLE root; - - RtlOpenCurrentUser( KEY_ALL_ACCESS, &root ); - attr.Length = sizeof(attr); - attr.RootDirectory = root; - attr.ObjectName = &nameW; - attr.Attributes = 0; - attr.SecurityDescriptor = NULL; - attr.SecurityQualityOfService = NULL; - RtlInitUnicodeString( &nameW, DllOverridesW ); - - /* @@ Wine registry key: HKCU\Software\Wine\DllOverrides */ - if (NtOpenKey( &std_key, KEY_ALL_ACCESS, &attr )) std_key = 0; - NtClose( root ); - } + RtlOpenCurrentUser( KEY_ALL_ACCESS, &root ); + memset(&attr, 0, sizeof(attr)); + attr.Length = sizeof(attr); + attr.RootDirectory = root; + attr.ObjectName = &nameW; + + /* @@ Wine registry key: HKCU\Software\Wine[<subkey>] */ + if (NtOpenKey( &std_key, KEY_ALL_ACCESS, &attr )) std_key = 0; + NtClose( root ); + + if(str) RtlFreeHeap( GetProcessHeap(), 0, str ); return std_key; }
@@ -328,26 +332,26 @@ static HANDLE get_standard_key(void) * * Get the registry key for the app-specific DllOverrides list. */ -static HANDLE get_app_key( const WCHAR *app_name ) +HANDLE get_app_key( const WCHAR *app_name, const WCHAR *subkey ) { OBJECT_ATTRIBUTES attr; UNICODE_STRING nameW; - HANDLE root; + HANDLE root, app_key; WCHAR *str; static const WCHAR AppDefaultsW[] = {'S','o','f','t','w','a','r','e','\','W','i','n','e','\', 'A','p','p','D','e','f','a','u','l','t','s','\',0}; - static const WCHAR DllOverridesW[] = {'\','D','l','l','O','v','e','r','r','i','d','e','s',0}; - static HANDLE app_key = (HANDLE)-1; + int size; + + app_name = get_basename( app_name );
- if (app_key != (HANDLE)-1) return app_key; + size = sizeof(AppDefaultsW) + strlenW(app_name) * sizeof(WCHAR); + if(subkey) size += strlenW(subkey) * sizeof(WCHAR);
- str = RtlAllocateHeap( GetProcessHeap(), 0, - sizeof(AppDefaultsW) + sizeof(DllOverridesW) + - strlenW(app_name) * sizeof(WCHAR) ); + str = RtlAllocateHeap( GetProcessHeap(), HEAP_ZERO_MEMORY, size); if (!str) return 0; strcpyW( str, AppDefaultsW ); strcatW( str, app_name ); - strcatW( str, DllOverridesW ); + if(subkey) strcatW( str, subkey );
RtlOpenCurrentUser( KEY_ALL_ACCESS, &root ); attr.Length = sizeof(attr); @@ -358,7 +362,7 @@ static HANDLE get_app_key( const WCHAR * attr.SecurityQualityOfService = NULL; RtlInitUnicodeString( &nameW, str );
- /* @@ Wine registry key: HKCU\Software\Wine\AppDefaults\app.exe\DllOverrides */ + /* @@ Wine registry key: HKCU\Software\Wine\AppDefaults\app.exe[<subkey>] */ if (NtOpenKey( &app_key, KEY_ALL_ACCESS, &attr )) app_key = 0; NtClose( root ); RtlFreeHeap( GetProcessHeap(), 0, str ); @@ -432,15 +436,17 @@ static enum loadorder get_load_order_val */ enum loadorder get_load_order( const WCHAR *app_name, const WCHAR *path ) { + const static WCHAR DllOverridesW[] = {'\','D','l','l','O','v','e','r','r','i','d','e','s',0}; enum loadorder ret = LO_INVALID; - HANDLE std_key, app_key = 0; + static HANDLE std_key = (HANDLE)-1, app_key = (HANDLE) -1; WCHAR *module, *basename; UNICODE_STRING path_str; int len;
if (!init_done) init_load_order(); - std_key = get_standard_key(); - if (app_name) app_key = get_app_key( app_name ); + if ( std_key == (HANDLE)-1 ) std_key = get_standard_key( DllOverridesW ); + if ( app_name && app_key == (HANDLE)-1 ) + app_key = get_app_key( app_name, DllOverridesW );
TRACE("looking for %s\n", debugstr_w(path));
diff --git a/dlls/ntdll/ntdll_misc.h b/dlls/ntdll/ntdll_misc.h index 1225036..310b528 100644 --- a/dlls/ntdll/ntdll_misc.h +++ b/dlls/ntdll/ntdll_misc.h @@ -144,6 +144,12 @@ struct debug_info char output[1024]; /* current output line */ };
+HANDLE get_standard_key( const WCHAR *subkey ); +HANDLE get_app_key( const WCHAR *app_name, const WCHAR *subkey ); + +void init_noexec_setting(const WCHAR *fullpath); + + struct ntdll_thread_data { struct debug_info *debug_info; /* info for debugstr functions */ diff --git a/dlls/ntdll/thread.c b/dlls/ntdll/thread.c diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c index c469f76..48859eb 100644 --- a/dlls/ntdll/virtual.c +++ b/dlls/ntdll/virtual.c @@ -51,6 +51,7 @@ #include "winioctl.h" #include "wine/library.h" #include "wine/server.h" +#include "wine/unicode.h" #include "wine/list.h" #include "wine/debug.h" #include "ntdll_misc.h" @@ -143,6 +144,7 @@ static void *user_space_limit = USER_SPA static void *preload_reserve_start; static void *preload_reserve_end; static int use_locks; +static BOOL read_implies_exec = FALSE; /* default to Off */
/*********************************************************************** @@ -460,7 +462,7 @@ static int VIRTUAL_GetUnixProt( BYTE vpr int prot = 0; if ((vprot & VPROT_COMMITTED) && !(vprot & VPROT_GUARD)) { - if (vprot & VPROT_READ) prot |= PROT_READ; + if (vprot & VPROT_READ) prot |= ( read_implies_exec ) ? PROT_READ | PROT_EXEC : PROT_READ; if (vprot & VPROT_WRITE) prot |= PROT_WRITE; if (vprot & VPROT_WRITECOPY) prot |= PROT_WRITE; if (vprot & VPROT_EXEC) prot |= PROT_EXEC; @@ -2020,3 +2022,56 @@ NTSTATUS WINAPI NtWriteVirtualMemory( HA if (bytes_written) *bytes_written = size; return status; } + +/* from heap.c */ +#define HEAP_DEF_SIZE 0x110000 /* Default heap size = 1Mb + 64Kb */ + +static int get_noexec_setting(HANDLE key, UNICODE_STRING *valueW) +{ + const static WCHAR onW[] = {'O','n',0}; + const static WCHAR offW[] = {'O','f','f',0}; + WCHAR buffer[10]; + DWORD count; + int ret = 0; + + if(!key) return 0; + + if( NtQueryValueKey( key, valueW, KeyValuePartialInformation, + buffer, sizeof(buffer), &count ) == STATUS_SUCCESS) { + WCHAR *str = (WCHAR *)((KEY_VALUE_PARTIAL_INFORMATION *)buffer)->Data; + + ret = 1; + if(strncmpW(str, onW, count) == 0) { + read_implies_exec = TRUE; + /* fix stack */ + mprotect( NtCurrentTeb()->Tib.StackLimit, + (size_t) NtCurrentTeb()->Tib.StackBase - (size_t)NtCurrentTeb()->Tib.StackLimit, + PROT_READ | PROT_WRITE | PROT_EXEC); + + /* fix heap (FIXME: this is wrong, should only fix already commited memory) */ + mprotect( NtCurrentTeb()->Peb->ProcessHeap, HEAP_DEF_SIZE, PROT_READ | PROT_WRITE | PROT_EXEC); + } + else if(strncmpW(str, offW, count)) { + ERR("invalid registry value for NoExecCompat: %s\n", debugstr_w(str)); + ret = 0; + } + } + + NtClose(key); + return ret; +} + +void init_noexec_setting(const WCHAR *app_name) +{ + static const WCHAR NoExecCompatW[] = { 'N','o','E','x','e','c','C','o','m','p','a','t',0}; + UNICODE_STRING valueW; + HANDLE key; + + RtlInitUnicodeString( &valueW, NoExecCompatW); + + key = get_app_key(app_name, NULL); + if(get_noexec_setting(key, &valueW)) return; + + key = get_standard_key(NULL); + get_noexec_setting(key, &valueW); +}