0

According to windows API docs we can use global variable to pass data from creating thread to the new thread and I am assuming the opposite is also possible

Data can also be passed from the creating thread to the new thread using global variables.

Here is a data structure and a callback function, ptr is a pointer to heap allocated memory in main

typedef struct Output
{
    char *ptr;
    DWORD len;

}Output, *POutput;

Output out; // global variable 


DWORD grab_output(LPVOID args)
{ 
    DWORD dread;
    BOOL success = FALSE;
    while (1)
    {
        success = ReadFile(g_hChildStd_OUT_Rd,out.ptr, 1024, &dread, NULL);
        if (!success || dread == 0 ) break;
        out.ptr = realloc(out.ptr, out.len+1024);
        out.len += dread;
    }   
}


int run()
{
    BOOL res; 
    STARTUPINFO si;
    PROCESS_INFORMATION pi;
    SECURITY_ATTRIBUTES sa;
    DWORD   dwThreadIdArray[1];


    
    DWORD n_size; 
    memset(&si, 0 ,sizeof(si));
    memset(&pi, 0, sizeof(pi));
    memset(&sa, 0, sizeof(sa));
    sa.nLength = sizeof(SECURITY_ATTRIBUTES);
    sa.bInheritHandle = TRUE;
    sa.lpSecurityDescriptor = NULL;

    if (!CreatePipe(&g_hChildStd_OUT_Rd, &g_hChildStd_OUT_Wr, &sa, 0)) 
        return GetLastError();
    
    if (!SetHandleInformation(g_hChildStd_OUT_Rd, HANDLE_FLAG_INHERIT, 0))
        return GetLastError();

    si.cb = sizeof(STARTUPINFOA);
    si.hStdError = g_hChildStd_OUT_Wr;
    si.hStdOutput = g_hChildStd_OUT_Wr; 
    si.dwFlags |= STARTF_USESTDHANDLES;

    if(!CreateProcess(NULL,
                        "C:\\Windows\\System32\\cmd.exe /c dir",
                        NULL,
                        NULL,               
                        TRUE,               
                        CREATE_NEW_CONSOLE,   
                        NULL,                
                        NULL,                                      
                        &si,                
                        &pi                 
                        ))
                        {
    }
    else
    {
        
        handle = CreateThread(0, 0, grab_output, NULL, 0, NULL);
    }
    return 0;
}


int main()
{
    out.ptr = malloc(1024);
    out.len = 0;
    run();
    printf("%s\n", out.ptr);

}

when running the code out.ptr returns garbage values

gcc example.c && ./a.exe 
└e╔┐═☺

for the sake of this question assume that I will be running a single thread at any given time

loaded_dypper
  • 262
  • 3
  • 12
  • @mikyll98 please explain why – loaded_dypper Sep 02 '22 at 13:44
  • 3
    Do you wait anywhere to allow the thread to fill the variable at all? How do you create that thread and what do you do then before you reach that `printf`? – Gerhardh Sep 02 '22 at 13:44
  • 1
    Remember that in C strings are really called ***null-terminated** strings*. And also, how do you synchronize between the thread and the `main` function to know when there is actual data in the buffer to write? How do you protect against data-races? – Some programmer dude Sep 02 '22 at 13:45
  • 1
    Did you read about synchronization between threads? Something like `volatile` or `atomic` ? – Gerhardh Sep 02 '22 at 13:45
  • @Someprogrammerdude I am learning from the docs takes time to realize when or where my program is vulnerable if you could explain how a data-race could happen in this code I would really appreciate it – loaded_dypper Sep 02 '22 at 13:49
  • 2
    Neither `volatile` nor `atomic`'s are about thread synchronization. The former instructs the compiler to not make assumptions about a particular value, and the latter makes sure the code adheres to the platform's memory model. Thread synchronization is done through calls to `WaitForSingleObject`, for example. Anyway, the primary thread never waits for the launched thread to signal that new values are available, so that's the issue here. – IInspectable Sep 02 '22 at 13:49
  • @Gerhardh I am using `CreateProcess` and create a thread using `CreateThread` which calls that callback function – loaded_dypper Sep 02 '22 at 13:53
  • Please do not describe what you are doing. Instead show it. – Gerhardh Sep 02 '22 at 13:55
  • @IInspectable [this](https://stackoverflow.com/questions/42402673/createprocess-and-capture-stdout) question has an answer that does not use `WaitForSingleObject` – loaded_dypper Sep 02 '22 at 13:56
  • @Gerhardh there you go the exact function on how am I creating the thread – loaded_dypper Sep 02 '22 at 13:58
  • @loaded_dypper Yes, that would explain why my comment says *"for example"*. – IInspectable Sep 02 '22 at 15:14

2 Answers2

1

The reason this prints garbage values is you assumed for no clear reason that the thread finishes before you accessed the global variable.

There's too many unchecked fault points. Right now, check every function that can error for an error return and produce output in that case, and also set a flag in another global variable. Really, you shouldn't have to, but better to much than not enough. Then close the process and thread handles you aren't using.

Now we should be able to discuss synchronization.

It looks like you want to grab all of the output at once. Thus WaitForSingleObject() on your own thread handle. To produce output incrementally, you need to track input highwater and output highwater and output only the characters in between with putc().

Don't forget to null-terminate your string either, or printf() won't be happy.

Joshua
  • 40,822
  • 8
  • 72
  • 132
  • Because of the `while(1)` in the thread it will never quit, however. So waiting for the thread handle won't work unless the thread callback is rewritten. – Lundin Sep 02 '22 at 14:08
  • This might not ne the best solution but i am still learning, I used `WaitForSingleObject` right after `CreateThread` and i now get the full output – loaded_dypper Sep 02 '22 at 14:15
  • @Lundin: But it will quit. `if (!success || dread == 0 ) break;` – Joshua Sep 02 '22 at 14:17
  • Oh true. Crappy indention strikes again. Anyway, it's probably not a good idea regardless, since there's ReadFileEx which could be used for the same purpose without even using threads. – Lundin Sep 02 '22 at 14:20
  • 1
    Although, what happens to a thread callback if it fails to `return`... and what happens to main() waiting for the thread handle then? That's actually a severe bug. – Lundin Sep 02 '22 at 14:21
  • @Lundin: Since we are not on itanium, the thread exit code is set to garbage and the program continues anyway. – Joshua Sep 02 '22 at 14:24
1

You need some means to communicate between the thread and main() that the value has been updated. It is very likely that main() will execute the printf before the thread is done with ReadFile. Also you have a race condition where ReadFile might be writing to the buffer while printf is reading it.

Use a mutex, semaphore, event or similar to communcate between threads, then use WaitForSingleObject etc where appropriate. It's also not advisable to have a busy-loop inside the thread or to exit the creator thread main() while there are threads still running.

EDIT:
Note that you must also return 0 from the thread or your program will go haywire, if you somehow managed to get it compiling in the first place despite the missing return.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • 1
    You are correct; but I was presented with a case of OP isn't quite there yet. Once he has the busy loop we can teach how to get rid of it. – Joshua Sep 02 '22 at 14:19