[PATCH v3 1/3] ntdll/tests: Test that ptids are multiples of four.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- v3: Add some NtOpenProcess / NtOpenThread tests. This only tries to open processes using the current process ptid with low bits sets, as it seems to be enough to give us the check we need. On Windows XP Pro, it returns STATUS_ACCESS_DENIED but on all other Windows versions it works and the low ptid bits are ignored. dlls/ntdll/tests/info.c | 57 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/dlls/ntdll/tests/info.c b/dlls/ntdll/tests/info.c index 8dc8bad645f6..a899353f8f9c 100644 --- a/dlls/ntdll/tests/info.c +++ b/dlls/ntdll/tests/info.c @@ -319,6 +319,11 @@ static void test_query_process(void) int i = 0, k = 0; BOOL is_nt = FALSE; SYSTEM_BASIC_INFORMATION sbi; + PROCESS_BASIC_INFORMATION pbi; + THREAD_BASIC_INFORMATION tbi; + OBJECT_ATTRIBUTES attr; + CLIENT_ID cid; + HANDLE handle; /* Copy of our winternl.h structure turned into a private one */ typedef struct _SYSTEM_PROCESS_INFORMATION_PRIVATE { @@ -401,13 +406,21 @@ static void test_query_process(void) if (!is_nt) { + DWORD_PTR tid; DWORD j; + + todo_wine_if(last_pid & 3) + ok(!(last_pid & 3), "Unexpected PID low bits: %p\n", spi->UniqueProcessId); for ( j = 0; j < spi->dwThreadCount; j++) { k++; ok ( spi->ti[j].ClientId.UniqueProcess == spi->UniqueProcessId, "The owning pid of the thread (%p) doesn't equal the pid (%p) of the process\n", spi->ti[j].ClientId.UniqueProcess, spi->UniqueProcessId); + + tid = (DWORD_PTR)spi->ti[j].ClientId.UniqueThread; + todo_wine_if(tid & 3) + ok(!(tid & 3), "Unexpected TID low bits: %p\n", spi->ti[j].ClientId.UniqueThread); } } @@ -423,6 +436,50 @@ static void test_query_process(void) if (one_before_last_pid == 0) one_before_last_pid = last_pid; HeapFree( GetProcessHeap(), 0, spi_buf); + + if (is_nt) + { + win_skip("skipping ptids low bits tests\n"); + return; + } + + for (i = 1; i < 4; ++i) + { + InitializeObjectAttributes( &attr, NULL, 0, NULL, NULL ); + cid.UniqueProcess = ULongToHandle(GetCurrentProcessId() + i); + cid.UniqueThread = 0; + + status = NtOpenProcess( &handle, PROCESS_QUERY_LIMITED_INFORMATION, &attr, &cid ); + todo_wine_if( status != STATUS_SUCCESS ) + ok( status == STATUS_SUCCESS, "NtOpenProcess returned:%x\n", status ); + if (status != STATUS_SUCCESS) continue; + + status = pNtQueryInformationProcess( handle, ProcessBasicInformation, &pbi, sizeof(pbi), NULL ); + ok( status == STATUS_SUCCESS, "NtQueryInformationProcess returned:%x\n", status ); + ok( pbi.UniqueProcessId == GetCurrentProcessId(), + "Expected pid %p, got %p\n", ULongToHandle(GetCurrentProcessId()), ULongToHandle(pbi.UniqueProcessId) ); + + NtClose( handle ); + } + + for (i = 1; i < 4; ++i) + { + InitializeObjectAttributes( &attr, NULL, 0, NULL, NULL ); + cid.UniqueProcess = 0; + cid.UniqueThread = ULongToHandle(GetCurrentThreadId() + i); + + status = NtOpenThread( &handle, THREAD_QUERY_LIMITED_INFORMATION, &attr, &cid ); + todo_wine_if( status != STATUS_SUCCESS ) + ok( status == STATUS_SUCCESS, "NtOpenThread returned:%x\n", status ); + if (status != STATUS_SUCCESS) continue; + + status = pNtQueryInformationThread( handle, ThreadBasicInformation, &tbi, sizeof(tbi), NULL ); + ok( status == STATUS_SUCCESS, "NtQueryInformationThread returned:%x\n", status ); + ok( tbi.ClientId.UniqueThread == ULongToHandle(GetCurrentThreadId()), + "Expected tid %p, got %p\n", ULongToHandle(GetCurrentThreadId()), tbi.ClientId.UniqueThread ); + + NtClose( handle ); + } } static void test_query_procperf(void) -- 2.26.1
This is kept separate from the previous patch to make it clear it works as expected on other Windows versions. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/tests/info.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/tests/info.c b/dlls/ntdll/tests/info.c index a899353f8f9c..1576f5ca971c 100644 --- a/dlls/ntdll/tests/info.c +++ b/dlls/ntdll/tests/info.c @@ -451,7 +451,8 @@ static void test_query_process(void) status = NtOpenProcess( &handle, PROCESS_QUERY_LIMITED_INFORMATION, &attr, &cid ); todo_wine_if( status != STATUS_SUCCESS ) - ok( status == STATUS_SUCCESS, "NtOpenProcess returned:%x\n", status ); + ok( status == STATUS_SUCCESS || broken( status == STATUS_ACCESS_DENIED ) /* wxppro */, + "NtOpenProcess returned:%x\n", status ); if (status != STATUS_SUCCESS) continue; status = pNtQueryInformationProcess( handle, ProcessBasicInformation, &pbi, sizeof(pbi), NULL ); @@ -470,7 +471,8 @@ static void test_query_process(void) status = NtOpenThread( &handle, THREAD_QUERY_LIMITED_INFORMATION, &attr, &cid ); todo_wine_if( status != STATUS_SUCCESS ) - ok( status == STATUS_SUCCESS, "NtOpenThread returned:%x\n", status ); + ok( status == STATUS_SUCCESS || broken( status == STATUS_ACCESS_DENIED ) /* wxppro */, + "NtOpenThread returned:%x\n", status ); if (status != STATUS_SUCCESS) continue; status = pNtQueryInformationThread( handle, ThreadBasicInformation, &tbi, sizeof(tbi), NULL ); -- 2.26.1
See: https://devblogs.microsoft.com/oldnewthing/?p=23283 Street Fighter V unpacker relies on it when validating other processes for its anti-debug checks, it uses (PID&0xfffffffc)>>2 as an array index and then checks back indexes against PIDs, and terminates early if some PIDs do not match. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/tests/info.c | 4 ---- server/process.c | 31 ++++++++++++++++++------------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/dlls/ntdll/tests/info.c b/dlls/ntdll/tests/info.c index 1576f5ca971c..e28eefa986d7 100644 --- a/dlls/ntdll/tests/info.c +++ b/dlls/ntdll/tests/info.c @@ -409,7 +409,6 @@ static void test_query_process(void) DWORD_PTR tid; DWORD j; - todo_wine_if(last_pid & 3) ok(!(last_pid & 3), "Unexpected PID low bits: %p\n", spi->UniqueProcessId); for ( j = 0; j < spi->dwThreadCount; j++) { @@ -419,7 +418,6 @@ static void test_query_process(void) spi->ti[j].ClientId.UniqueProcess, spi->UniqueProcessId); tid = (DWORD_PTR)spi->ti[j].ClientId.UniqueThread; - todo_wine_if(tid & 3) ok(!(tid & 3), "Unexpected TID low bits: %p\n", spi->ti[j].ClientId.UniqueThread); } } @@ -450,7 +448,6 @@ static void test_query_process(void) cid.UniqueThread = 0; status = NtOpenProcess( &handle, PROCESS_QUERY_LIMITED_INFORMATION, &attr, &cid ); - todo_wine_if( status != STATUS_SUCCESS ) ok( status == STATUS_SUCCESS || broken( status == STATUS_ACCESS_DENIED ) /* wxppro */, "NtOpenProcess returned:%x\n", status ); if (status != STATUS_SUCCESS) continue; @@ -470,7 +467,6 @@ static void test_query_process(void) cid.UniqueThread = ULongToHandle(GetCurrentThreadId() + i); status = NtOpenThread( &handle, THREAD_QUERY_LIMITED_INFORMATION, &attr, &cid ); - todo_wine_if( status != STATUS_SUCCESS ) ok( status == STATUS_SUCCESS || broken( status == STATUS_ACCESS_DENIED ) /* wxppro */, "NtOpenThread returned:%x\n", status ); if (status != STATUS_SUCCESS) continue; diff --git a/server/process.c b/server/process.c index 73984f363f59..211207ed03b6 100644 --- a/server/process.c +++ b/server/process.c @@ -339,21 +339,24 @@ static void kill_all_processes(void); #define PTID_OFFSET 8 /* offset for first ptid value */ +static unsigned int index_from_ptid(unsigned int id) { return id / 4; } +static unsigned int ptid_from_index(unsigned int index) { return index * 4; } + /* allocate a new process or thread id */ unsigned int alloc_ptid( void *ptr ) { struct ptid_entry *entry; - unsigned int id; + unsigned int index; if (used_ptid_entries < alloc_ptid_entries) { - id = used_ptid_entries + PTID_OFFSET; + index = used_ptid_entries + PTID_OFFSET; entry = &ptid_entries[used_ptid_entries++]; } else if (next_free_ptid && num_free_ptids >= 256) { - id = next_free_ptid; - entry = &ptid_entries[id - PTID_OFFSET]; + index = next_free_ptid; + entry = &ptid_entries[index - PTID_OFFSET]; if (!(next_free_ptid = entry->next)) last_free_ptid = 0; num_free_ptids--; } @@ -368,35 +371,37 @@ unsigned int alloc_ptid( void *ptr ) } ptid_entries = entry; alloc_ptid_entries = count; - id = used_ptid_entries + PTID_OFFSET; + index = used_ptid_entries + PTID_OFFSET; entry = &ptid_entries[used_ptid_entries++]; } entry->ptr = ptr; - return id; + return ptid_from_index( index ); } /* free a process or thread id */ void free_ptid( unsigned int id ) { - struct ptid_entry *entry = &ptid_entries[id - PTID_OFFSET]; + unsigned int index = index_from_ptid( id ); + struct ptid_entry *entry = &ptid_entries[index - PTID_OFFSET]; entry->ptr = NULL; entry->next = 0; /* append to end of free list so that we don't reuse it too early */ - if (last_free_ptid) ptid_entries[last_free_ptid - PTID_OFFSET].next = id; - else next_free_ptid = id; - last_free_ptid = id; + if (last_free_ptid) ptid_entries[last_free_ptid - PTID_OFFSET].next = index; + else next_free_ptid = index; + last_free_ptid = index; num_free_ptids++; } /* retrieve the pointer corresponding to a process or thread id */ void *get_ptid_entry( unsigned int id ) { - if (id < PTID_OFFSET) return NULL; - if (id - PTID_OFFSET >= used_ptid_entries) return NULL; - return ptid_entries[id - PTID_OFFSET].ptr; + unsigned int index = index_from_ptid( id ); + if (index < PTID_OFFSET) return NULL; + if (index - PTID_OFFSET >= used_ptid_entries) return NULL; + return ptid_entries[index - PTID_OFFSET].ptr; } /* return the main thread of the process */ -- 2.26.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=70500 Your paranoid android. === wxppro (32 bit report) === ntdll: info.c:454: Test failed: NtOpenProcess returned:c0000022 info.c:454: Test failed: NtOpenProcess returned:c0000022 info.c:454: Test failed: NtOpenProcess returned:c0000022 info.c:473: Test failed: NtOpenThread returned:c0000022 info.c:473: Test failed: NtOpenThread returned:c0000022 info.c:473: Test failed: NtOpenThread returned:c0000022
participants (2)
-
Marvin -
Rémi Bernon