[PATCH v2 1/2] ntdll/tests: Test that ptids are multiples of four.

[Rémi Bernon]

Signed-off-by: Rémi Bernon 
---
 dlls/ntdll/tests/info.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/dlls/ntdll/tests/info.c b/dlls/ntdll/tests/info.c
index 8dc8bad645f6..91408e213310 100644
--- a/dlls/ntdll/tests/info.c
+++ b/dlls/ntdll/tests/info.c
@@ -401,13 +401,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);
             }
         }

[PATCH v2 2/2] server: Make sure ptids are multiples of four.

[Rémi Bernon]

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 
---
 dlls/ntdll/tests/info.c |  2 --
 server/process.c        | 31 ++++++++++++++++++-------------
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/dlls/ntdll/tests/info.c b/dlls/ntdll/tests/info.c
index 91408e213310..a02444702025 100644
--- a/dlls/ntdll/tests/info.c
+++ b/dlls/ntdll/tests/info.c
@@ -404,7 +404,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++) 
             {
@@ -414,7 +413,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);
             }
         }
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 */

[Alexandre Julliard]

Rémi Bernon  writes:
> Signed-off-by: Rémi Bernon
>
> dlls/ntdll/tests/info.c | 8 ++++++++
>  1 file changed, 8 insertions(+)

I meant testing APIs that take ptids, like OpenProcess(). Your patch will
ignore the lower bits, but that may not be the right thing to do.

[Rémi Bernon]

On 4/24/20 9:17 PM, Alexandre Julliard wrote:
> Rémi Bernon  writes:
> > Signed-off-by: Rémi Bernon
> >
> > dlls/ntdll/tests/info.c | 8 ++++++++
> >   1 file changed, 8 insertions(+)
>
> I meant testing APIs that take ptids, like OpenProcess(). Your patch will
> ignore the lower bits, but that may not be the right thing to do.

Ah right, I'll do some more tests.