[PATCH] ntoskrnl: Have MmIsAddressValid use IsBadReadPtr.

[Thomas Faber]

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.