Pass non-dynamic memory allocation to CreateThread() function

Viewed 36

I pass a address of USB_INSTACE_DATA variable to function CreateThread() like this. Is it safe to use, or I must use dynamic memory allocation for USB_INSTACE_DATA data?

DWORD WINAPI MyThreadFunction(LPVOID lpParam);
void GetUSBInfo(PDEV_BROADCAST_DEVICEINTERFACE pDevInf, WPARAM wParam) {

    USB_INSTACE_DATA data{ pDevInf, wParam };
    DWORD   dwThreadId;
    auto hThreadArray = CreateThread(
        NULL,                   // default security attributes
        0,                      // use default stack size  
        MyThreadFunction,       // thread function name
        &data,                  // argument to thread function 
        0,                      // use default creation flags 
        &dwThreadId);           // returns the thread identifier
}
1 Answers

This isn't safe. USB_INSTACE_DATA data{ pDevInf, wParam }; has automatic storage duration. Once the enclosing scope ends (i.e. when the function returns), the memory is cleaned up.

The thread launched through CreateThread, however, holds a pointer &data beyond the function's scope. This is an instance of the classical use-after-free bug.

The solution is to make data live at least as long as any client holding a reference/pointer to it. You can use memory with static storage duration, or manual memory management (such as heap allocations). Mind you, this doesn't automagically solve any other issues inherent to concurrent programs such as dealing with data races. That's an issue you will have to address separately.

Related