make check_no_exec() work reliable

Peter Beutner p.beutner at gmx.net
Fri Nov 24 05:01:56 CST 2006


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.

---
 dlls/kernel32/except.c   |   39 ++-------------------------------------
 dlls/ntdll/signal_i386.c |   37 +++++++++++++++++++++++++++++++++++--
 2 files changed, 37 insertions(+), 39 deletions(-)

diff --git a/dlls/kernel32/except.c b/dlls/kernel32/except.c
index 725e4ec..af3da97 100644
--- a/dlls/kernel32/except.c
+++ b/dlls/kernel32/except.c
@@ -412,33 +412,6 @@ inline static BOOL check_resource_write(
 
 
 /*******************************************************************
- *         check_no_exec
- *
- * Check for executing a protected area.
- */
-inline static BOOL check_no_exec( void *addr )
-{
-    MEMORY_BASIC_INFORMATION info;
-
-    if (!VirtualQuery( addr, &info, sizeof(info) )) return FALSE;
-    if (info.State == MEM_FREE) return FALSE;
-
-    /* prot |= PAGE_EXECUTE would be a lot easier, but MS developers
-     * apparently don't grasp the notion of protection bits */
-    switch(info.Protect)
-    {
-    case PAGE_READONLY: info.Protect = PAGE_EXECUTE_READ; break;
-    case PAGE_READWRITE: info.Protect = PAGE_EXECUTE_READWRITE; break;
-    case PAGE_WRITECOPY: info.Protect = PAGE_EXECUTE_WRITECOPY; break;
-    default: return FALSE;
-    }
-    /* FIXME: we should probably have a per-app option, and maybe a message box */
-    FIXME( "No-exec fault triggered at %p, enabling work-around\n", addr );
-    return VirtualProtect( addr, 1, info.Protect, NULL );
-}
-
-
-/*******************************************************************
  *         UnhandledExceptionFilter   (KERNEL32.@)
  */
 LONG WINAPI UnhandledExceptionFilter(PEXCEPTION_POINTERS epointers)
@@ -447,17 +420,9 @@ LONG WINAPI UnhandledExceptionFilter(PEX
 
     if (rec->ExceptionCode == EXCEPTION_ACCESS_VIOLATION && rec->NumberParameters >= 2)
     {
-        switch(rec->ExceptionInformation[0])
-        {
-        case EXCEPTION_WRITE_FAULT:
-            if (check_resource_write( (void *)rec->ExceptionInformation[1] ))
+        if( (rec->ExceptionInformation[0] == EXCEPTION_WRITE_FAULT)
+            && check_resource_write( (void *)rec->ExceptionInformation[1] ) )
                 return EXCEPTION_CONTINUE_EXECUTION;
-            break;
-        case EXCEPTION_EXECUTE_FAULT:
-            if (check_no_exec( (void *)rec->ExceptionInformation[1] ))
-                return EXCEPTION_CONTINUE_EXECUTION;
-            break;
-        }
     }
 
     if (!NtCurrentTeb()->Peb->BeingDebugged)
diff --git a/dlls/ntdll/signal_i386.c b/dlls/ntdll/signal_i386.c
index 8166124..a75e6eb 100644
--- a/dlls/ntdll/signal_i386.c
+++ b/dlls/ntdll/signal_i386.c
@@ -850,6 +850,37 @@ static BOOL check_atl_thunk( EXCEPTION_R
     return ret;
 }
 
+/***********************************************************************
+ *              check_no_exec
+ * Check for executing a protected area.
+ */
+static BOOL check_no_exec( void *addr )
+{
+    MEMORY_BASIC_INFORMATION info;
+    SIZE_T ret, size=1;
+
+    if( NtQueryVirtualMemory( GetCurrentProcess(), addr, MemoryBasicInformation,
+                              &info, sizeof(info), &ret))
+        return FALSE;
+
+    if(info.State == MEM_FREE) return FALSE;
+
+    /* prot |= PAGE_EXECUTE would be a lot easier, but MS developers
+     * apparently don't grasp the notion of protection bits */
+    switch(info.Protect)
+    {
+    case PAGE_READONLY: info.Protect = PAGE_EXECUTE_READ; break;
+    case PAGE_READWRITE: info.Protect = PAGE_EXECUTE_READWRITE; break;
+    case PAGE_WRITECOPY: info.Protect = PAGE_EXECUTE_WRITECOPY; break;
+    default: return FALSE;
+    }
+    /* FIXME: we should probably have a per-app option, and maybe a message box */
+    FIXME( "No-exec fault triggered at %p, enabling work-around\n", addr );
+    if( NtProtectVirtualMemory( GetCurrentProcess(), &addr, &size, info.Protect, NULL) )
+        return FALSE;
+
+    return TRUE;
+}
 
 /***********************************************************************
  *           setup_exception
@@ -983,8 +1014,10 @@ static void WINAPI raise_segv_exception(
     case EXCEPTION_ACCESS_VIOLATION:
         if (rec->NumberParameters == 2)
         {
-            if (rec->ExceptionInformation[0] == EXCEPTION_EXECUTE_FAULT && check_atl_thunk( rec, context ))
-                goto done;
+            if (rec->ExceptionInformation[0] == EXCEPTION_EXECUTE_FAULT) {
+                if( check_atl_thunk( rec, context ) ||
+                    check_no_exec( (void *)rec->ExceptionInformation[1]) )  goto done;
+            }
             rec->ExceptionCode = VIRTUAL_HandleFault( (void *)rec->ExceptionInformation[1] );
         }
         break;
-- 
1.4.3.4





More information about the wine-patches mailing list