[PATCH] ntoskrnl.exe: fix handling relocations on page boundary

Rafał H rafalh92 at outlook.com
Wed Jan 2 11:29:40 CST 2019


Code is only used for non-page-aligned device drivers.
Previously if relocation was happening on page boundary it crashed
because only one page had changed protection in load_driver_module.
Now code is changing protection for two pages (current and next).
Also added "goto error" for VirtualProtect call for the current page. This
change is safe because if it failed before this change updating the page
content would crash anyway.
Commit fixes running MTA: San Andreas game.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46401
Signed-off-by: Rafał Harabień <rafalh92 at outlook.com>
---
 dlls/ntoskrnl.exe/ntoskrnl.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c
index 48b309f3d6..0c32e12801 100644
--- a/dlls/ntoskrnl.exe/ntoskrnl.c
+++ b/dlls/ntoskrnl.exe/ntoskrnl.c
@@ -3318,8 +3318,9 @@ static HMODULE load_driver_module( const WCHAR *name )
     if (nt->OptionalHeader.SectionAlignment < info.PageSize ||
         !(nt->FileHeader.Characteristics & IMAGE_FILE_DLL))
     {
-        DWORD old;
+        DWORD old, old_next_page;
         IMAGE_BASE_RELOCATION *rel, *end;
+        void *page, *next_page = NULL;
 
         if ((rel = RtlImageDirectoryEntryToData( module, TRUE, IMAGE_DIRECTORY_ENTRY_BASERELOC, &size )))
         {
@@ -3327,13 +3328,33 @@ static HMODULE load_driver_module( const WCHAR *name )
             end = (IMAGE_BASE_RELOCATION *)((char *)rel + size);
             while (rel < end && rel->SizeOfBlock)
             {
-                void *page = (char *)module + rel->VirtualAddress;
-                VirtualProtect( page, info.PageSize, PAGE_EXECUTE_READWRITE, &old );
+                page = (char *)module + rel->VirtualAddress;
+                /* if the page protection was already changed in previous interation don't do it again here */
+                if (page == next_page)
+                    old = old_next_page;
+                else
+                {
+                    /* bring back old protection for 'next_page' from previous iteration */
+                    if (old_next_page != PAGE_EXECUTE_READWRITE)
+                        VirtualProtect( next_page, info.PageSize, old_next_page, &old_next_page );
+                    /* change current page protection */
+                    if (!VirtualProtect( page, info.PageSize, PAGE_EXECUTE_READWRITE, &old ))
+                        goto error;
+                }
+
+                /* protect the next page as well because relocation can occur on the page boundary */
+                next_page = (char*)page + info.PageSize;
+                if (!VirtualProtect( next_page, info.PageSize, PAGE_EXECUTE_READWRITE, &old_next_page ))
+                    next_page = NULL;
+
                 rel = LdrProcessRelocationBlock( page, (rel->SizeOfBlock - sizeof(*rel)) / sizeof(USHORT),
                                                  (USHORT *)(rel + 1), delta );
+                /* bring back old protection of current page; next page is handled in the next iteration */
                 if (old != PAGE_EXECUTE_READWRITE) VirtualProtect( page, info.PageSize, old, &old );
                 if (!rel) goto error;
             }
+            if (next_page && old_next_page != PAGE_EXECUTE_READWRITE)
+                VirtualProtect( next_page, info.PageSize, old_next_page, &old_next_page );
             /* make sure we don't try again */
             size = FIELD_OFFSET( IMAGE_NT_HEADERS, OptionalHeader ) + nt->FileHeader.SizeOfOptionalHeader;
             VirtualProtect( nt, size, PAGE_READWRITE, &old );
-- 
2.17.1



More information about the wine-devel mailing list