[PATCH v2 0/3] MR8612: ntdll: Return STATUS_INVALID_ADDRESS for free regions in get_memory_region_info().
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58531 -- v2: ntdll: Fill RegionSize as view size in get_memory_region_info(). https://gitlab.winehq.org/wine/wine/-/merge_requests/8612
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/tests/virtual.c | 70 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index c48652f0f65..56aa48c4de1 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -298,6 +298,7 @@ static void check_region_size_(void *p, SIZE_T s, unsigned int line) static void test_NtAllocateVirtualMemoryEx(void) { + MEMORY_REGION_INFORMATION mri; MEMORY_BASIC_INFORMATION mbi; MEM_EXTENDED_PARAMETER ext[2]; char *p, *p1, *p2, *p3; @@ -615,6 +616,35 @@ static void test_NtAllocateVirtualMemoryEx(void) ok(p2 == p1 + size / 2, "Unexpected addr %p, expected %p.\n", p2, p1 + size / 2); check_region_size(p1, size / 2); check_region_size(p2, size / 2); + + status = NtQueryVirtualMemory( NtCurrentProcess(), p1, MemoryBasicInformation, &mbi, sizeof(mbi), NULL ); + ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status ); + ok( mbi.AllocationBase == p1, "got %p.\n", mbi.AllocationBase ); + ok( mbi.Type == MEM_PRIVATE, "got %#lx.\n", mbi.Type ); + ok( mbi.State == MEM_RESERVE, "got %#lx.\n", mbi.State ); + ok( mbi.RegionSize == size / 2, "Unexpected size %Iu, expected %Iu.\n", mbi.RegionSize, size / 2 ); + ok( mbi.AllocationProtect == PAGE_NOACCESS, "got %#lx.\n", mbi.AllocationProtect ); + status = NtQueryVirtualMemory( NtCurrentProcess(), p1, MemoryRegionInformation, &mri, sizeof(mri), NULL ); + ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status ); + ok( mri.AllocationBase == p1, "got %p.\n", mri.AllocationBase ); + ok( mri.RegionSize == size / 2, "Unexpected size %Iu, expected %Iu.\n", mri.RegionSize, size / 2 ); + ok( !mri.CommitSize, "Unexpected size %Iu.\n", mri.CommitSize ); + ok( mri.AllocationProtect == PAGE_NOACCESS, "got %#lx.\n", mri.AllocationProtect ); + + status = NtQueryVirtualMemory( NtCurrentProcess(), p2, MemoryBasicInformation, &mbi, sizeof(mbi), NULL ); + ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status ); + ok( mbi.AllocationBase == p2, "got %p.\n", mbi.AllocationBase ); + ok( mbi.Type == MEM_PRIVATE, "got %#lx.\n", mbi.Type ); + ok( mbi.State == MEM_RESERVE, "got %#lx.\n", mbi.State ); + ok( mbi.RegionSize == size / 2, "Unexpected size %Iu, expected %Iu.\n", mbi.RegionSize, size / 2 ); + ok( mbi.AllocationProtect == PAGE_NOACCESS, "got %#lx.\n", mbi.AllocationProtect ); + status = NtQueryVirtualMemory( NtCurrentProcess(), p2, MemoryRegionInformation, &mri, sizeof(mri), NULL ); + ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status ); + ok( mri.AllocationBase == p2, "got %p.\n", mri.AllocationBase ); + ok( mri.RegionSize == size / 2, "Unexpected size %Iu, expected %Iu.\n", mri.RegionSize, size / 2 ); + ok( !mri.CommitSize, "Unexpected size %Iu.\n", mri.CommitSize ); + ok( mri.AllocationProtect == PAGE_NOACCESS, "got %#lx.\n", mri.AllocationProtect ); + status = NtFreeVirtualMemory(NtCurrentProcess(), (void **)&p1, &size2, MEM_RELEASE); ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status); ok(size2 == 0x8000, "Unexpected size %#Ix.\n", size2); @@ -2633,7 +2663,8 @@ static void test_query_region_information(void) SIZE_T len, size; NTSTATUS status; HANDLE mapping; - void *ptr; + void *ptr, *addr; + ULONG old; size = 0x10000; ptr = NULL; @@ -2689,10 +2720,45 @@ static void test_query_region_information(void) ok(!info.DirectMapped, "Unexpected flag %d.\n", info.DirectMapped); ok(info.RegionSize == size, "Unexpected region size.\n"); + addr = (char *)ptr + 0x1000; + size = 0x1000; + status = NtProtectVirtualMemory(NtCurrentProcess(), &addr, &size, PAGE_NOACCESS, &old ); + ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status); + status = NtQueryVirtualMemory(NtCurrentProcess(), ptr, MemoryRegionInformation, &info, sizeof(info), &len); + ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status); + ok(info.AllocationBase == ptr, "Unexpected base %p.\n", info.AllocationBase); + ok(info.AllocationProtect == PAGE_READWRITE, "Unexpected protection %lu.\n", info.AllocationProtect); + ok(!info.Private, "Unexpected flag %d.\n", info.Private); + ok(!info.MappedDataFile, "Unexpected flag %d.\n", info.MappedDataFile); + ok(!info.MappedImage, "Unexpected flag %d.\n", info.MappedImage); + ok(!info.MappedPageFile, "Unexpected flag %d.\n", info.MappedPageFile); + ok(!info.MappedPhysical, "Unexpected flag %d.\n", info.MappedPhysical); + ok(!info.DirectMapped, "Unexpected flag %d.\n", info.DirectMapped); + todo_wine ok(info.RegionSize == 0x10000, "Unexpected region size %#Ix.\n", info.RegionSize); + + status = NtQueryVirtualMemory(NtCurrentProcess(), (char *)ptr + 0x1000, MemoryRegionInformation, &info, sizeof(info), &len); + ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status); + ok(info.AllocationBase == ptr, "Unexpected base %p.\n", info.AllocationBase); + ok(info.AllocationProtect == PAGE_READWRITE, "Unexpected protection %lu.\n", info.AllocationProtect); + ok(!info.Private, "Unexpected flag %d.\n", info.Private); + ok(!info.MappedDataFile, "Unexpected flag %d.\n", info.MappedDataFile); + ok(!info.MappedImage, "Unexpected flag %d.\n", info.MappedImage); + ok(!info.MappedPageFile, "Unexpected flag %d.\n", info.MappedPageFile); + ok(!info.MappedPhysical, "Unexpected flag %d.\n", info.MappedPhysical); + ok(!info.DirectMapped, "Unexpected flag %d.\n", info.DirectMapped); + todo_wine ok(info.RegionSize == 0x10000, "Unexpected region size %#Ix.\n", info.RegionSize); + size = 0; status = NtFreeVirtualMemory(NtCurrentProcess(), &ptr, &size, MEM_RELEASE); ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status); + memset(&info, 0xcc, sizeof(info)); + status = NtQueryVirtualMemory(NtCurrentProcess(), ptr, MemoryRegionInformation, &info, sizeof(info), &len); + todo_wine ok(status == STATUS_INVALID_ADDRESS, "Unexpected status %08lx.\n", status); + todo_wine ok(info.AllocationBase == (void *)(ULONG_PTR)0xcccccccccccccccc, "got %p.\n", info.AllocationBase); + todo_wine ok(info.AllocationProtect == 0xcccccccc, "Unexpected protection %lu.\n", info.AllocationProtect); + todo_wine ok(info.RegionType == 0xcccccccc, "got %#lx.\n", info.RegionType); + /* Pagefile mapping */ mapping = CreateFileMappingA(INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE, 0, 4096, NULL); ok(mapping != 0, "CreateFileMapping failed\n"); @@ -2840,7 +2906,7 @@ static void test_query_image_information(void) status = NtMapViewOfSection( mapping, NtCurrentProcess(), &ptr, 0, 0, &offset, &size, 1, 0, PAGE_READONLY ); ok( status == STATUS_IMAGE_NOT_AT_BASE, "Unexpected status %08lx\n", status ); todo_wine - ok( size == 0x4000, "wrong size %Ix\n", size ); + todo_wine ok( size == 0x4000, "wrong size %Ix\n", size ); NtClose( mapping ); nt = RtlImageNtHeader( ptr ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8612
From: Paul Gofman <pgofman(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58531 --- dlls/ntdll/tests/virtual.c | 8 ++++---- dlls/ntdll/unix/virtual.c | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 56aa48c4de1..a8fe5b7ed40 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -2754,10 +2754,10 @@ static void test_query_region_information(void) memset(&info, 0xcc, sizeof(info)); status = NtQueryVirtualMemory(NtCurrentProcess(), ptr, MemoryRegionInformation, &info, sizeof(info), &len); - todo_wine ok(status == STATUS_INVALID_ADDRESS, "Unexpected status %08lx.\n", status); - todo_wine ok(info.AllocationBase == (void *)(ULONG_PTR)0xcccccccccccccccc, "got %p.\n", info.AllocationBase); - todo_wine ok(info.AllocationProtect == 0xcccccccc, "Unexpected protection %lu.\n", info.AllocationProtect); - todo_wine ok(info.RegionType == 0xcccccccc, "got %#lx.\n", info.RegionType); + ok(status == STATUS_INVALID_ADDRESS, "Unexpected status %08lx.\n", status); + ok(info.AllocationBase == (void *)(ULONG_PTR)0xcccccccccccccccc, "got %p.\n", info.AllocationBase); + ok(info.AllocationProtect == 0xcccccccc, "Unexpected protection %lu.\n", info.AllocationProtect); + ok(info.RegionType == 0xcccccccc, "got %#lx.\n", info.RegionType); /* Pagefile mapping */ mapping = CreateFileMappingA(INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE, 0, 4096, NULL); diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index e733e3ffdd6..246539940cd 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -5585,6 +5585,7 @@ static unsigned int get_memory_region_info( HANDLE process, LPCVOID addr, MEMORY } if ((status = fill_basic_memory_info( addr, &basic_info ))) return status; + if (basic_info.State == MEM_FREE) return STATUS_INVALID_ADDRESS; info->AllocationBase = basic_info.AllocationBase; info->AllocationProtect = basic_info.AllocationProtect; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8612
From: Paul Gofman <pgofman(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58531 --- dlls/ntdll/tests/virtual.c | 4 ++-- dlls/ntdll/unix/virtual.c | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index a8fe5b7ed40..b103f264e63 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -2734,7 +2734,7 @@ static void test_query_region_information(void) ok(!info.MappedPageFile, "Unexpected flag %d.\n", info.MappedPageFile); ok(!info.MappedPhysical, "Unexpected flag %d.\n", info.MappedPhysical); ok(!info.DirectMapped, "Unexpected flag %d.\n", info.DirectMapped); - todo_wine ok(info.RegionSize == 0x10000, "Unexpected region size %#Ix.\n", info.RegionSize); + ok(info.RegionSize == 0x10000, "Unexpected region size %#Ix.\n", info.RegionSize); status = NtQueryVirtualMemory(NtCurrentProcess(), (char *)ptr + 0x1000, MemoryRegionInformation, &info, sizeof(info), &len); ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status); @@ -2746,7 +2746,7 @@ static void test_query_region_information(void) ok(!info.MappedPageFile, "Unexpected flag %d.\n", info.MappedPageFile); ok(!info.MappedPhysical, "Unexpected flag %d.\n", info.MappedPhysical); ok(!info.DirectMapped, "Unexpected flag %d.\n", info.DirectMapped); - todo_wine ok(info.RegionSize == 0x10000, "Unexpected region size %#Ix.\n", info.RegionSize); + ok(info.RegionSize == 0x10000, "Unexpected region size %#Ix.\n", info.RegionSize); size = 0; status = NtFreeVirtualMemory(NtCurrentProcess(), &ptr, &size, MEM_RELEASE); diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 246539940cd..64a60eca24a 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -5407,7 +5407,7 @@ NTSTATUS WINAPI NtProtectVirtualMemory( HANDLE process, PVOID *addr_ptr, SIZE_T } -static unsigned int fill_basic_memory_info( const void *addr, MEMORY_BASIC_INFORMATION *info ) +static unsigned int fill_basic_memory_info( const void *addr, MEMORY_BASIC_INFORMATION *info, ULONG_PTR *view_size ) { char *base, *alloc_base = 0, *alloc_end = working_set_limit; struct wine_rb_entry *ptr; @@ -5439,6 +5439,7 @@ static unsigned int fill_basic_memory_info( const void *addr, MEMORY_BASIC_INFOR { alloc_base = view->base; alloc_end = (char *)view->base + view->size; + if (view_size) *view_size = view->size; break; } } @@ -5563,7 +5564,7 @@ static unsigned int get_basic_memory_info( HANDLE process, LPCVOID addr, return result.virtual_query.status; } - if ((status = fill_basic_memory_info( addr, info ))) return status; + if ((status = fill_basic_memory_info( addr, info, NULL ))) return status; if (res_len) *res_len = sizeof(*info); return STATUS_SUCCESS; @@ -5573,6 +5574,7 @@ static unsigned int get_memory_region_info( HANDLE process, LPCVOID addr, MEMORY SIZE_T len, SIZE_T *res_len ) { MEMORY_BASIC_INFORMATION basic_info; + ULONG_PTR view_size = 0; unsigned int status; if (len < FIELD_OFFSET(MEMORY_REGION_INFORMATION, CommitSize)) @@ -5584,14 +5586,14 @@ static unsigned int get_memory_region_info( HANDLE process, LPCVOID addr, MEMORY return STATUS_NOT_IMPLEMENTED; } - if ((status = fill_basic_memory_info( addr, &basic_info ))) return status; + if ((status = fill_basic_memory_info( addr, &basic_info, &view_size ))) return status; if (basic_info.State == MEM_FREE) return STATUS_INVALID_ADDRESS; info->AllocationBase = basic_info.AllocationBase; info->AllocationProtect = basic_info.AllocationProtect; info->RegionType = 0; /* FIXME */ if (len >= FIELD_OFFSET(MEMORY_REGION_INFORMATION, CommitSize)) - info->RegionSize = basic_info.RegionSize; + info->RegionSize = view_size; if (len >= FIELD_OFFSET(MEMORY_REGION_INFORMATION, PartitionId)) info->CommitSize = basic_info.State == MEM_COMMIT ? basic_info.RegionSize : 0; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8612
I know it's the same type alias at the end, but SIZE_T looks more suitable for a size value. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8612#note_110542
Dmitry Timoshkov (@dmitry) commented about dlls/ntdll/unix/virtual.c:
return STATUS_NOT_IMPLEMENTED; }
- if ((status = fill_basic_memory_info( addr, &basic_info ))) return status; + if ((status = fill_basic_memory_info( addr, &basic_info, &view_size ))) return status; + if (basic_info.State == MEM_FREE) return STATUS_INVALID_ADDRESS;
info->AllocationBase = basic_info.AllocationBase; info->AllocationProtect = basic_info.AllocationProtect; info->RegionType = 0; /* FIXME */ if (len >= FIELD_OFFSET(MEMORY_REGION_INFORMATION, CommitSize)) - info->RegionSize = basic_info.RegionSize; + info->RegionSize = view_size;
Wouldn't it be more natural to use SIZE_T for view_size since info-\>RegionSize has this type? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8612#note_110543
On Mon Jul 21 19:08:31 2025 +0000, Dmitry Timoshkov wrote:
Wouldn't it be more natural to use SIZE_T for view_size since info-\>RegionSize has this type? Yes, thanks, that is an oversight.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8612#note_110546
On Mon Jul 21 19:32:55 2025 +0000, Paul Gofman wrote:
Yes, thanks, that is an oversight. ... but it looks like CommitSize is also wrong actually, so I will look into the details, maybe it is better to implement almost entirely separately then.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8612#note_110560
participants (4)
-
Dmitry Timoshkov (＠dmitry) -
Nikolay Sivov (＠nsivov)
-
Paul Gofman
-
Paul Gofman (＠gofman)