Fixes an error inside EasyAntiCheat.sys, which uses MmIsAddressValid on ntoskrnl.exe, to find its base address. Wine will always return FALSE, because it checks for write access to ntoskrnl, when all the driver needs is read access
Tested on Arch Linux
Signed-off-by: Derek Lesho dereklesho52@Gmail.com --- dlls/ntoskrnl.exe/ntoskrnl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c index 55027c5..e4c1c35 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/ntoskrnl.c @@ -2164,7 +2164,7 @@ void WINAPI MmFreeNonCachedMemory( void *addr, SIZE_T size ) BOOLEAN WINAPI MmIsAddressValid(PVOID VirtualAddress) { TRACE("(%p)\n", VirtualAddress); - return !IsBadWritePtr(VirtualAddress, 1); + return !IsBadReadPtr(VirtualAddress, 1); }
/***********************************************************************
Side note, while this change does not follow the MSDN, it appears that the MSDN incorrectly documents the function.
This blog post from 2006 explains more: https://blogs.msdn.microsoft.com/doronh/2006/03/09/beware-the-shiny-light-th...
On Tue, Jun 5, 2018 at 11:14 AM, Derek Lesho dereklesho52@gmail.com wrote:
Fixes an error inside EasyAntiCheat.sys, which uses MmIsAddressValid on ntoskrnl.exe, to find its base address. Wine will always return FALSE, because it checks for write access to ntoskrnl, when all the driver needs is read access
Tested on Arch Linux
Signed-off-by: Derek Lesho dereklesho52@Gmail.com
dlls/ntoskrnl.exe/ntoskrnl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c index 55027c5..e4c1c35 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/ntoskrnl.c @@ -2164,7 +2164,7 @@ void WINAPI MmFreeNonCachedMemory( void *addr, SIZE_T size ) BOOLEAN WINAPI MmIsAddressValid(PVOID VirtualAddress) { TRACE("(%p)\n", VirtualAddress);
- return !IsBadWritePtr(VirtualAddress, 1);
- return !IsBadReadPtr(VirtualAddress, 1);
}
/***********************************************************************
2.7.4
Hi Derek,
I don't think MSDN incorrectly documents this function. The function is meant to be used in nt kernel space, however wineserver is in user space as far as I know. So we would need different implementation.
May be we could do something like return !IsBadWritePtr(VirtualAddress, 1) || !IsBadReadPtr(VirtualAddress, 1). And write a test to verify such behavior.
Regards, Zhiyi
On Tue 6 5 23:35, Derek Lesho wrote:
Side note, while this change does not follow the MSDN, it appears that the MSDN incorrectly documents the function.
This blog post from 2006 explains more: https://blogs.msdn.microsoft.com/doronh/2006/03/09/beware-the-shiny-light-th...
On Tue, Jun 5, 2018 at 11:14 AM, Derek Lesho dereklesho52@gmail.com wrote:
Fixes an error inside EasyAntiCheat.sys, which uses MmIsAddressValid on ntoskrnl.exe, to find its base address. Wine will always return FALSE, because it checks for write access to ntoskrnl, when all the driver needs is read access
Tested on Arch Linux
Signed-off-by: Derek Lesho dereklesho52@Gmail.com
dlls/ntoskrnl.exe/ntoskrnl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c index 55027c5..e4c1c35 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/ntoskrnl.c @@ -2164,7 +2164,7 @@ void WINAPI MmFreeNonCachedMemory( void *addr, SIZE_T size ) BOOLEAN WINAPI MmIsAddressValid(PVOID VirtualAddress) { TRACE("(%p)\n", VirtualAddress);
- return !IsBadWritePtr(VirtualAddress, 1);
- return !IsBadReadPtr(VirtualAddress, 1);
}
/***********************************************************************
2.7.4
On 2018-06-06 04:36, Zhiyi Zhang wrote:
The function is meant to be used in nt kernel space, however wineserver is in user space as far as I know. So we would need different implementation.
May be we could do something like return !IsBadWritePtr(VirtualAddress, 1) || !IsBadReadPtr(VirtualAddress, 1). And write a test to verify such behavior.
IsBadReadPtr(x) implies IsBadWritePtr(x), since Windows does not have a concept of "write-only" access to pages. So checking both seems unnecessary. The real difference between IsBadReadPtr and MmIsAddressValid is that the former can actually _make_ the address valid by causing a page fault (and paging in or zeroing the page). MmIsAddressValid on the other hand would simply look at the current state of the page table entry without side effects.
This stuff isn't hard to test per se, but a full test that's correct for Windows kernel land will not currently work in Wine due to unimplemented Mm stuff, e.g.
for (protection in [PAGE_NOACCESS, PAGE_READONLY, PAGE_READWRITE]) { ZwAllocateVirtualMemory(..., &Base, ..., MEM_COMMIT, protection); // physical page is not assigned until first access todo_wine ok(!MmIsAddressValid(Base, 1), ...); // force a physical page to appear and prevent it from getting paged out mdl = IoAllocateMdl(Base, 1, FALSE, FALSE, NULL); __TRY { MmProbeAndLockPages(mdl, UserMode, IoModifyAccess); todo_wine ok(protection != PAGE_NOACCESS, ...); } __EXCEPT_PAGE_FAULT { ok(protection == PAGE_NOACCESS, ...); goto Next; } // the system address is now guaranteed to be valid todo_wine ok(MmIsAddressValid(MmGetSystemAddressForMdlSafe(mdl, NormalPagePriority), 1), ...); MmUnlockPages... Next: IoFreeMdl... ZwFreeVirtualMemory... }
... has a todo with pretty much every ok(), with or without the patch :\
With some luck, the simpler void test_function() { ok(MmIsAddressValid((void*)test_function, ...); // fails in master, succeeds with patch? ptr = ExAllocatePoolWithTag(NonPagedPool, ...) ok(MmIsAddressValid(ptr, ...); ExFreePoolWithTag... ok(!MmIsAddressValid(NULL), ...); }
might be enough to show that the patch is correct though.
Hi Thomas, in your example, does &Base refer to the base of ntoskrnl.exe? If so, why are you allocating a buffer there, wouldn't that corrupt the memory of ntoskrnl, or am I misunderstanding what your code is doing. On windows, do drivers have access to change the memory of ntoskrnl?
Atleast in EAC, they find the address of a function inside of the ntoskrnl address space, and continuously decrement the address until MMisAddressValid returns FALSE. Without my patch, wine always returns FALSE to EAC, even on the first address check.
On Wed, Jun 6, 2018 at 3:51 AM, Thomas Faber thomas.faber@reactos.org wrote:
On 2018-06-06 04:36, Zhiyi Zhang wrote:
The function is meant to be used in nt kernel space, however wineserver is in user space as far as I know. So we would need different implementation.
May be we could do something like return !IsBadWritePtr(VirtualAddress, 1) || !IsBadReadPtr(VirtualAddress, 1). And write a test to verify such behavior.
IsBadReadPtr(x) implies IsBadWritePtr(x), since Windows does not have a concept of "write-only" access to pages. So checking both seems unnecessary. The real difference between IsBadReadPtr and MmIsAddressValid is that the former can actually _make_ the address valid by causing a page fault (and paging in or zeroing the page). MmIsAddressValid on the other hand would simply look at the current state of the page table entry without side effects.
This stuff isn't hard to test per se, but a full test that's correct for Windows kernel land will not currently work in Wine due to unimplemented Mm stuff, e.g.
for (protection in [PAGE_NOACCESS, PAGE_READONLY, PAGE_READWRITE]) { ZwAllocateVirtualMemory(..., &Base, ..., MEM_COMMIT, protection); // physical page is not assigned until first access todo_wine ok(!MmIsAddressValid(Base, 1), ...); // force a physical page to appear and prevent it from getting paged out mdl = IoAllocateMdl(Base, 1, FALSE, FALSE, NULL); __TRY { MmProbeAndLockPages(mdl, UserMode, IoModifyAccess); todo_wine ok(protection != PAGE_NOACCESS, ...); } __EXCEPT_PAGE_FAULT { ok(protection == PAGE_NOACCESS, ...); goto Next; } // the system address is now guaranteed to be valid todo_wine ok(MmIsAddressValid(MmGetSystemAddressForMdlSafe(mdl, NormalPagePriority), 1), ...); MmUnlockPages... Next: IoFreeMdl... ZwFreeVirtualMemory... }
... has a todo with pretty much every ok(), with or without the patch :\
With some luck, the simpler void test_function() { ok(MmIsAddressValid((void*)test_function, ...); // fails in master, succeeds with patch? ptr = ExAllocatePoolWithTag(NonPagedPool, ...) ok(MmIsAddressValid(ptr, ...); ExFreePoolWithTag... ok(!MmIsAddressValid(NULL), ...); }
might be enough to show that the patch is correct though.
On 2018-06-06 17:11, Derek Lesho wrote:
Hi Thomas, in your example, does &Base refer to the base of ntoskrnl.exe? If so, why are you allocating a buffer there, wouldn't that corrupt the memory of ntoskrnl, or am I misunderstanding what your code is doing. On windows, do drivers have access to change the memory of ntoskrnl?
My intention was to simply make a user mode memory allocation, so there should be a "Base = NULL;" before the call to ZwAllocateVirtualMemory. This function cannot allocate kernel address space (or overwrite loaded modules), so it wouldn't do anything unexpected. I was just suggesting it to demonstrate behavior with NOACCESS/READONLY/READWRITE pages. As for your actual use case, I'd assume ntoskrnl's code is most likely write-protected on Windows as well, much like you encountered on Wine, but I haven't verified.
Since your patch was committed I don't know if you're still interested in adding the test. If you are, I'd suggest to go with the simpler second version, since like I said the "full test" won't be very useful in Wine.
On Wed, Jun 6, 2018 at 3:51 AM, Thomas Faber thomas.faber@reactos.org wrote:
void test_function() { ok(MmIsAddressValid((void*)test_function), ...); // fails in master, succeeds with patch? ptr = ExAllocatePoolWithTag(NonPagedPool, ...) ok(MmIsAddressValid(ptr), ...); ExFreePoolWithTag... ok(!MmIsAddressValid(NULL), ...); }
Perhaps also ok(MmIsAddressValid((void*)MmIsAddressValid), ...); to specifically test ntoskrnl instead of the test driver's binary.
-Thomas
-
Derek Lesho -
Thomas Faber -
Zhiyi Zhang