Fix a dangerous cross-thread memory access.

When a winhandl.c input thread returns EOF to the main thread, the
latter might immediately delete the input thread's context. I
carefully wrote in a comment that in that case we had to not touch ctx
ever again after signalling to the main thread - but the test for
whether that was true, which also touched ctx, itself came _after_ the
SetEvent which sent that signal. Ahem.

Spotted by Minefield, which it looks as if I haven't run for a while.

(cherry picked from commit 9fec2e773873e28f1409f5e1eefaf03483070050)

diff --git a/windows/winhandl.c b/windows/winhandl.c
index d81054f36414c355b205ae1a612ab86733e1d08d..7c282fc7d5502f128f300d3ea09d942026d4c525 100644 (file)
--- a/windows/winhandl.c
+++ b/windows/winhandl.c
@@ -115,7 +115,7 @@ static DWORD WINAPI handle_input_threadfunc(void *param)
     struct handle_input *ctx = (struct handle_input *) param;
     OVERLAPPED ovl, *povl;
     HANDLE oev;
-    int readret, readlen;
+    int readret, readlen, finished;
 
     if (ctx->flags & HANDLE_FLAG_OVERLAPPED) {
        povl = &ovl;
@@ -165,18 +165,20 @@ static DWORD WINAPI handle_input_threadfunc(void *param)
            (ctx->flags & HANDLE_FLAG_IGNOREEOF))
            continue;
 
+        /*
+         * If we just set ctx->len to 0, that means the read operation
+         * has returned end-of-file. Telling that to the main thread
+         * will cause it to set its 'defunct' flag and dispose of the
+         * handle structure at the next opportunity, in which case we
+         * mustn't touch ctx at all after the SetEvent. (Hence we do
+         * even _this_ check before the SetEvent.)
+         */
+        finished = (ctx->len == 0);
+
        SetEvent(ctx->ev_to_main);
 
-       if (!ctx->len) {
-            /*
-             * The read operation has returned end-of-file. Telling
-             * that to the main thread will cause it to set its
-             * 'defunct' flag and dispose of the handle structure at
-             * the next opportunity, so we must not touch ctx at all
-             * after this.
-             */
+       if (finished)
            break;
-        }
 
        WaitForSingleObject(ctx->ev_from_main, INFINITE);
        if (ctx->done) {