From 3cdeb6d0d29937677a82849208ea48827e1c28c6 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Sat, 15 Oct 2011 22:16:50 -0700 Subject: [PATCH] Fix page protection bug causing a rare startup crash (bug 5133, r=fyren). --- core-legacy/sourcehook/sh_memory.h | 123 ++++++++++++++++ core-legacy/sourcehook/sourcehook.cpp | 2 +- core/sourcehook/sh_memory.h | 147 ++++++++++++++++++++ core/sourcehook/sourcehook_impl_cvfnptr.cpp | 2 +- 4 files changed, 272 insertions(+), 2 deletions(-) diff --git a/core-legacy/sourcehook/sh_memory.h b/core-legacy/sourcehook/sh_memory.h index 3951b1a..8ec7536 100644 --- a/core-legacy/sourcehook/sh_memory.h +++ b/core-legacy/sourcehook/sh_memory.h @@ -47,6 +47,116 @@ namespace SourceHook { + static inline bool GetPageBits(void *addr, int *bits) + { +#if SH_SYS == SH_SYS_LINUX + // On linux, first check /proc/self/maps + unsigned long laddr = reinterpret_cast(addr); + + FILE *pF = fopen("/proc/self/maps", "r"); + if (pF) { + // Linux /proc/self/maps -> parse + // Format: + // lower upper prot stuff path + // 08048000-0804c000 r-xp 00000000 03:03 1010107 /bin/cat + unsigned long rlower, rupper; + char r, w, x; + while (fscanf(pF, "%lx-%lx %c%c%c", &rlower, &rupper, &r, &w, &x) != EOF) { + // Check whether we're IN THERE! + if (laddr >= rlower && laddr < rupper) { + fclose(pF); + *bits = 0; + if (r == 'r') + *bits |= SH_MEM_READ; + if (w == 'w') + *bits |= SH_MEM_WRITE; + if (x == 'x') + *bits |= SH_MEM_EXEC; + return true; + } + // Read to end of line + int c; + while ((c = fgetc(pF)) != '\n') { + if (c == EOF) + break; + } + if (c == EOF) + break; + } + fclose(pF); + return false; + } + pF = fopen("/proc/curproc/map", "r"); + if (pF) { + // FreeBSD /proc/curproc/map -> parse + // 0x804800 0x805500 13 15 0xc6e18960 r-x 21 0x0 COW NC vnode + unsigned long rlower, rupper, ignoreLong; + int ignoreInt; + char r, w, x; + while (fscanf(pF, "0x%lx 0x%lx %d %d 0x%lx %c%c%c", &rlower, &rupper, &ignoreInt, + &ignoreInt, &ignoreLong, &r, &w, &x) != EOF) { + // Check whether we're IN THERE! + if (laddr >= rlower && laddr < rupper) { + fclose(pF); + *bits = 0; + if (r == 'r') + *bits |= SH_MEM_READ; + if (r == 'w') + *bits |= SH_MEM_WRITE; + if (r == 'x') + *bits |= SH_MEM_EXEC; + return true; + } + // Read to end of line + int c; + while ((c = fgetc(pF)) != '\n') { + if (c == EOF) + break; + } + if (c == EOF) + break; + } + fclose(pF); + return false; + } + return false; +#elif SH_SYS == SH_SYS_APPLE +#elif SH_XP == SH_XP_WINAPI + SYSTEM_INFO info; + GetSystemInfo(&info); + + MEMORY_BASIC_INFORMATION mem; + size_t base = size_t(addr) & ~size_t(info.dwPageSize - 1); + if (!VirtualQuery((void *)base, &mem, sizeof(mem))) + return false; + switch (mem.Protect) { + case PAGE_EXECUTE: + *bits = SH_MEM_EXEC; + break; + case PAGE_EXECUTE_READ: + *bits = SH_MEM_EXEC | SH_MEM_READ; + break; + case PAGE_EXECUTE_READWRITE: + case PAGE_EXECUTE_WRITECOPY: + *bits = SH_MEM_EXEC | SH_MEM_READ | SH_MEM_WRITE; + break; + case PAGE_NOACCESS: + *bits = 0; + break; + case PAGE_READONLY: + *bits = SH_MEM_READ; + break; + case PAGE_READWRITE: + case PAGE_WRITECOPY: + *bits = SH_MEM_READ | SH_MEM_WRITE; + break; + default: + return false; + } + return true; +#endif + } + inline bool SetMemAccess(void *addr, size_t len, int access) { # if SH_XP == SH_XP_POSIX @@ -70,6 +180,19 @@ namespace SourceHook # endif } + inline bool MakePageWritable(void *addr) + { + int bits; + if (GetPageBits(addr, &bits)) { + if (bits & SH_MEM_WRITE) + return true; + bits |= SH_MEM_WRITE; + } else { + bits = SH_MEM_READ | SH_MEM_WRITE | SH_MEM_EXEC; + } + return SetMemAccess(addr, sizeof(void *), bits); + } + #if SH_XP == SH_XP_POSIX namespace { diff --git a/core-legacy/sourcehook/sourcehook.cpp b/core-legacy/sourcehook/sourcehook.cpp index e2fc8b0..40c4861 100644 --- a/core-legacy/sourcehook/sourcehook.cpp +++ b/core-legacy/sourcehook/sourcehook.cpp @@ -366,7 +366,7 @@ namespace SourceHook CVfnPtr vfp(cur_vfnptr, &m_OneIgnore); // Alter vtable entry - if (!SetMemAccess(cur_vtptr, sizeof(void*) * (tmp.m_VtblIdx + 1), SH_MEM_READ | SH_MEM_WRITE)) + if (!MakePageWritable(cur_vtptr)) return 0; *reinterpret_cast(cur_vfnptr) = *reinterpret_cast(hookman->m_HookfuncVfnptr); diff --git a/core/sourcehook/sh_memory.h b/core/sourcehook/sh_memory.h index f3e03d0..9ffa1fb 100644 --- a/core/sourcehook/sh_memory.h +++ b/core/sourcehook/sh_memory.h @@ -43,11 +43,145 @@ # else # error Unsupported OS/Compiler # endif +#if SH_SYS == SH_SYS_APPLE +# include +# include +# include +#endif #include "sh_list.h" namespace SourceHook { + static inline bool GetPageBits(void *addr, int *bits) + { +#if SH_SYS == SH_SYS_LINUX + // On linux, first check /proc/self/maps + unsigned long laddr = reinterpret_cast(addr); + + FILE *pF = fopen("/proc/self/maps", "r"); + if (pF) { + // Linux /proc/self/maps -> parse + // Format: + // lower upper prot stuff path + // 08048000-0804c000 r-xp 00000000 03:03 1010107 /bin/cat + unsigned long rlower, rupper; + char r, w, x; + while (fscanf(pF, "%lx-%lx %c%c%c", &rlower, &rupper, &r, &w, &x) != EOF) { + // Check whether we're IN THERE! + if (laddr >= rlower && laddr < rupper) { + fclose(pF); + *bits = 0; + if (r == 'r') + *bits |= SH_MEM_READ; + if (w == 'w') + *bits |= SH_MEM_WRITE; + if (x == 'x') + *bits |= SH_MEM_EXEC; + return true; + } + // Read to end of line + int c; + while ((c = fgetc(pF)) != '\n') { + if (c == EOF) + break; + } + if (c == EOF) + break; + } + fclose(pF); + return false; + } + pF = fopen("/proc/curproc/map", "r"); + if (pF) { + // FreeBSD /proc/curproc/map -> parse + // 0x804800 0x805500 13 15 0xc6e18960 r-x 21 0x0 COW NC vnode + unsigned long rlower, rupper, ignoreLong; + int ignoreInt; + char r, w, x; + while (fscanf(pF, "0x%lx 0x%lx %d %d 0x%lx %c%c%c", &rlower, &rupper, &ignoreInt, + &ignoreInt, &ignoreLong, &r, &w, &x) != EOF) { + // Check whether we're IN THERE! + if (laddr >= rlower && laddr < rupper) { + fclose(pF); + *bits = 0; + if (r == 'r') + *bits |= SH_MEM_READ; + if (r == 'w') + *bits |= SH_MEM_WRITE; + if (r == 'x') + *bits |= SH_MEM_EXEC; + return true; + } + // Read to end of line + int c; + while ((c = fgetc(pF)) != '\n') { + if (c == EOF) + break; + } + if (c == EOF) + break; + } + fclose(pF); + return false; + } + return false; +#elif SH_SYS == SH_SYS_APPLE + vm_size_t ignoreSize; + vm_address_t vmaddr = (vm_address_t)addr; + vm_region_basic_info_data_t info; + vm_region_flavor_t flavor = VM_REGION_BASIC_INFO; + memory_object_name_t obj; + + mach_msg_type_number_t count = VM_REGION_BASIC_INFO_COUNT; + kern_return_t kr = vm_region(mach_task_self(), &vmaddr, &ignoreSize, flavor, + (vm_region_info_t)&info, &count, &obj); + if (kr != KERN_SUCCESS) + return false; + *bits = 0; + if (info.protection & VM_PROT_READ) + *bits |= SH_MEM_READ; + if (info.protection & VM_PROT_WRITE) + *bits |= SH_MEM_WRITE; + if (info.protection & VM_PROT_EXECUTE) + *bits |= SH_MEM_EXEC; + return true; +#elif SH_XP == SH_XP_WINAPI + SYSTEM_INFO info; + GetSystemInfo(&info); + + MEMORY_BASIC_INFORMATION mem; + size_t base = size_t(addr) & ~size_t(info.dwPageSize - 1); + if (!VirtualQuery((void *)base, &mem, sizeof(mem))) + return false; + switch (mem.Protect) { + case PAGE_EXECUTE: + *bits = SH_MEM_EXEC; + break; + case PAGE_EXECUTE_READ: + *bits = SH_MEM_EXEC | SH_MEM_READ; + break; + case PAGE_EXECUTE_READWRITE: + case PAGE_EXECUTE_WRITECOPY: + *bits = SH_MEM_EXEC | SH_MEM_READ | SH_MEM_WRITE; + break; + case PAGE_NOACCESS: + *bits = 0; + break; + case PAGE_READONLY: + *bits = SH_MEM_READ; + break; + case PAGE_READWRITE: + case PAGE_WRITECOPY: + *bits = SH_MEM_READ | SH_MEM_WRITE; + break; + default: + return false; + } + return true; +#endif + } + inline bool SetMemAccess(void *addr, size_t len, int access) { # if SH_XP == SH_XP_POSIX @@ -71,6 +205,19 @@ namespace SourceHook # endif } + inline bool MakePageWritable(void *addr) + { + int bits; + if (GetPageBits(addr, &bits)) { + if (bits & SH_MEM_WRITE) + return true; + bits |= SH_MEM_WRITE; + } else { + bits = SH_MEM_READ | SH_MEM_WRITE | SH_MEM_EXEC; + } + return SetMemAccess(addr, sizeof(void *), bits); + } + #if SH_XP == SH_XP_POSIX namespace { diff --git a/core/sourcehook/sourcehook_impl_cvfnptr.cpp b/core/sourcehook/sourcehook_impl_cvfnptr.cpp index 5159b0c..65ea49a 100644 --- a/core/sourcehook/sourcehook_impl_cvfnptr.cpp +++ b/core/sourcehook/sourcehook_impl_cvfnptr.cpp @@ -217,7 +217,7 @@ namespace SourceHook bool CVfnPtr::Patch(void *newValue) { - if (!SetMemAccess(m_Ptr, sizeof(void*), SH_MEM_READ | SH_MEM_WRITE)) + if (!MakePageWritable(m_Ptr)) { return false; }