APP CRASH: (8.0 Master <custom_multithread_program) win32/syscall.c:thread_handle_to_pid() Thread Handle Leak on Windows

Created by: dkim-virsec

Describe the bug

In running a hello-world type multi-threaded program, I saw that when running this application (source attached) under DR, thread handles are leaked when the source application uses CreateThread on Windows platforms.

To Reproduce

1. Compile the multithread-example.cpp shown here:

#include <windows.h>
#include <iostream>

using namespace std;

DWORD WINAPI thread1(LPVOID lpParameter)
{
    unsigned int& size = *((unsigned int*)lpParameter);
    for (int i = 0; i < 20000000 * (rand()%100); i++)
    {
        if ((i%100000000 == 0))
        {
            cout << "thread 1 " << i << endl;
        }
    }
    return 0;
}

DWORD WINAPI thread2(LPVOID lpParameter)
{
    unsigned int& size = *((unsigned int*)lpParameter);
    for (int i = 0; i < 2000000000; i++)
    {
        if ((i % 10000000 == 0))
        {
            cout << "thread 2 " << i << endl;
        }
    }
    return 0;
}

int main(int argc, char* argv[])
{
    unsigned int s1 = 10;
    unsigned int s2 = 20;
    DWORD myThreadID[2];
    
    HANDLE myHandle[2];

    for (int i = 0; i < 2; i++)
    {
        myHandle[i] = CreateThread(0, 0, thread1, &s1, 0, myThreadID+i);
    }
    //system("pause");
    for (int i = 0; i < 2; i++)
    {
        WaitForSingleObject(myHandle[i], INFINITE);
    }
    for (int i = 0; i < 2; i++)
    {
        cout << CloseHandle(myHandle[i]) << endl;
    }
    system("pause");
    return 0;
}

2. When the execution is paused after CloseHandle() is called, view the open handles through any debugging tool (I used ProcessExplorer64.exe -> view handles) when running this application natively vs under DR.
3. When running under DR, there are extra thread handles that are not closed properly. The cause is found in win32/syscall.c:thread_handle_to_pid():

Versions

• What version of DynamoRIO are you using? 8.0 Master Build
• Does the latest build from https://github.com/DynamoRIO/dynamorio/wiki/Latest-Build solve the problem? No
• What operating system version are you running on? ("Windows 10" is not sufficient: give the release number.) Windows 2016 Version 1607 (OS Build 14393.2580)
• Is your application 32-bit or 64-bit? Occurs for both 32-bit and 64-bit builds

Proposed Solution

The problem is found in this function: win32/syscall.c:thread_handle_to_pid() When this function executes, it creates a thread_handle, but forgets to close it.

if (tid != INVALID_THREAD_ID) {
        // Get a handle with more privileges
        thread_handle = thread_handle_from_id(tid);
    }
    return process_id_from_thread_handle(thread_handle);

This should be:

    if (tid != INVALID_THREAD_ID) {
        /* Get a handle with more privileges */
        thread_handle = thread_handle_from_id(tid);
        process_id_t pid = process_id_from_thread_handle(thread_handle);
        close_handle(thread_handle);
        return pid;
    }
    return process_id_from_thread_handle(thread_handle);