Sebastian Lackner : ntdll: Improve check_atl_thunk to prevent passing exceptions to the usermode application.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: 34b2d920b47122007b65d435e064d018fb37b21f
URL:    http://source.winehq.org/git/wine.git/?a=commit;h=34b2d920b47122007b65d435e0...

Author: Sebastian Lackner <sebastian(a)fds-team.de>
Date:   Tue Oct 14 06:22:55 2014 +0200

ntdll: Improve check_atl_thunk to prevent passing exceptions to the usermode application.

---

 dlls/kernel32/tests/virtual.c |  1 -
 dlls/ntdll/ntdll_misc.h       |  2 ++
 dlls/ntdll/signal_i386.c      | 20 +++++------
 dlls/ntdll/virtual.c          | 78 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 89 insertions(+), 12 deletions(-)

diff --git a/dlls/kernel32/tests/virtual.c b/dlls/kernel32/tests/virtual.c
index d258190..73b753e 100644
--- a/dlls/kernel32/tests/virtual.c
+++ b/dlls/kernel32/tests/virtual.c
@@ -2071,7 +2071,6 @@ static void test_atl_thunk_emulation( ULONG dep_flags )
             pRtlRemoveVectoredExceptionHandler( vectored_handler );
 
             ok( ret == 43, "call returned wrong result, expected 43, got %d\n", ret );
-            todo_wine
             ok( num_execute_fault_calls == 1, "expected one STATUS_ACCESS_VIOLATION exception, got %d exceptions\n", num_execute_fault_calls );
         }
         else
diff --git a/dlls/ntdll/ntdll_misc.h b/dlls/ntdll/ntdll_misc.h
index 4370084..aac9320 100644
--- a/dlls/ntdll/ntdll_misc.h
+++ b/dlls/ntdll/ntdll_misc.h
@@ -169,6 +169,8 @@ extern BOOL virtual_is_valid_code_address( const void *addr, SIZE_T size ) DECLS
 extern NTSTATUS virtual_handle_fault( LPCVOID addr, DWORD err ) DECLSPEC_HIDDEN;
 extern BOOL virtual_check_buffer_for_read( const void *ptr, SIZE_T size ) DECLSPEC_HIDDEN;
 extern BOOL virtual_check_buffer_for_write( void *ptr, SIZE_T size ) DECLSPEC_HIDDEN;
+extern SIZE_T virtual_uninterrupted_read_memory( const void *addr, void *buffer, SIZE_T size ) DECLSPEC_HIDDEN;
+extern SIZE_T virtual_uninterrupted_write_memory( void *addr, const void *buffer, SIZE_T size ) DECLSPEC_HIDDEN;
 extern void VIRTUAL_SetForceExec( BOOL enable ) DECLSPEC_HIDDEN;
 extern void virtual_release_address_space(void) DECLSPEC_HIDDEN;
 extern void virtual_set_large_address_space(void) DECLSPEC_HIDDEN;
diff --git a/dlls/ntdll/signal_i386.c b/dlls/ntdll/signal_i386.c
index 7f01554..f97799c 100644
--- a/dlls/ntdll/signal_i386.c
+++ b/dlls/ntdll/signal_i386.c
@@ -1633,26 +1633,24 @@ struct atl_thunk
 static BOOL check_atl_thunk( EXCEPTION_RECORD *rec, CONTEXT *context )
 {
     const struct atl_thunk *thunk = (const struct atl_thunk *)rec->ExceptionInformation[1];
+    struct atl_thunk thunk_copy;
     BOOL ret = FALSE;
 
-    if (!virtual_is_valid_code_address( thunk, sizeof(*thunk) )) return FALSE;
+    if (virtual_uninterrupted_read_memory( thunk, &thunk_copy, sizeof(*thunk) ) != sizeof(*thunk))
+        return FALSE;
 
-    __TRY
+    if (thunk_copy.movl == 0x042444c7 && thunk_copy.jmp == 0xe9)
     {
-        if (thunk->movl == 0x042444c7 && thunk->jmp == 0xe9)
+        if (virtual_uninterrupted_write_memory( (DWORD *)context->Esp + 1,
+            &thunk_copy.this, sizeof(DWORD) ) == sizeof(DWORD))
         {
-            *((DWORD *)context->Esp + 1) = thunk->this;
-            context->Eip = (DWORD_PTR)(&thunk->func + 1) + thunk->func;
+            context->Eip = (DWORD_PTR)(&thunk->func + 1) + thunk_copy.func;
             TRACE( "emulating ATL thunk at %p, func=%08x arg=%08x\n",
-                   thunk, context->Eip, *((DWORD *)context->Esp + 1) );
+                   thunk, context->Eip, thunk_copy.this );
             ret = TRUE;
         }
     }
-    __EXCEPT_PAGE_FAULT
-    {
-        return FALSE;
-    }
-    __ENDTRY
+
     return ret;
 }
 
diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c
index d2bb152..4c4c05d 100644
--- a/dlls/ntdll/virtual.c
+++ b/dlls/ntdll/virtual.c
@@ -1672,6 +1672,84 @@ BOOL virtual_check_buffer_for_write( void *ptr, SIZE_T size )
 
 
 /***********************************************************************
+ *           virtual_uninterrupted_read_memory
+ *
+ * Similar to NtReadVirtualMemory, but without wineserver calls. Moreover
+ * permissions are checked before accessing each page, to ensure that no
+ * exceptions can happen.
+ */
+SIZE_T virtual_uninterrupted_read_memory( const void *addr, void *buffer, SIZE_T size )
+{
+    struct file_view *view;
+    sigset_t sigset;
+    SIZE_T bytes_read = 0;
+
+    if (!size) return 0;
+
+    server_enter_uninterrupted_section( &csVirtual, &sigset );
+    if ((view = VIRTUAL_FindView( addr, size )))
+    {
+        if (!(view->protect & VPROT_SYSTEM))
+        {
+            void *page = ROUND_ADDR( addr, page_mask );
+            BYTE *p = view->prot + (((const char *)page - (const char *)view->base) >> page_shift);
+
+            while (bytes_read < size && (VIRTUAL_GetUnixProt( *p++ ) & PROT_READ))
+            {
+                SIZE_T block_size = min( size, page_size - ((UINT_PTR)addr & page_mask) );
+                memcpy( buffer, addr, block_size );
+
+                addr   = (const void *)((const char *)addr + block_size);
+                buffer = (void *)((char *)buffer + block_size);
+                bytes_read += block_size;
+            }
+        }
+    }
+    server_leave_uninterrupted_section( &csVirtual, &sigset );
+    return bytes_read;
+}
+
+
+/***********************************************************************
+ *           virtual_uninterrupted_write_memory
+ *
+ * Similar to NtWriteVirtualMemory, but without wineserver calls. Moreover
+ * permissions are checked before accessing each page, to ensure that no
+ * exceptions can happen.
+ */
+SIZE_T virtual_uninterrupted_write_memory( void *addr, const void *buffer, SIZE_T size )
+{
+    struct file_view *view;
+    sigset_t sigset;
+    SIZE_T bytes_written = 0;
+
+    if (!size) return 0;
+
+    server_enter_uninterrupted_section( &csVirtual, &sigset );
+    if ((view = VIRTUAL_FindView( addr, size )))
+    {
+        if (!(view->protect & VPROT_SYSTEM))
+        {
+            void *page = ROUND_ADDR( addr, page_mask );
+            BYTE *p = view->prot + (((const char *)page - (const char *)view->base) >> page_shift);
+
+            while (bytes_written < size && (VIRTUAL_GetUnixProt( *p++ ) & PROT_WRITE))
+            {
+                SIZE_T block_size = min( size, page_size - ((UINT_PTR)addr & page_mask) );
+                memcpy( addr, buffer, block_size );
+
+                addr   = (void *)((char *)addr + block_size);
+                buffer = (const void *)((const char *)buffer + block_size);
+                bytes_written += block_size;
+            }
+        }
+    }
+    server_leave_uninterrupted_section( &csVirtual, &sigset );
+    return bytes_written;
+}
+
+
+/***********************************************************************
  *           VIRTUAL_SetForceExec
  *
  * Whether to force exec prot on all views.