Kai Jourdan spotted a rather embarrassing double-free, and Minefield

[u/mdw/putty] / windows / winhandl.c

diff --git a/windows/winhandl.c b/windows/winhandl.c
index a5b7c69..f515ee5 100644 (file)
--- a/windows/winhandl.c
+++ b/windows/winhandl.c
@@ -16,6 +16,17 @@
  * write; so the output thread waits for an event object notifying
  * it to _attempt_ a write, and then it sets an event in return
  * when one completes.
+ * 
+ * (It's terribly annoying having to spawn a subthread for each
+ * direction of each handle. Technically it isn't necessary for
+ * serial ports, since we could use overlapped I/O within the main
+ * thread and wait directly on the event objects in the OVERLAPPED
+ * structures. However, we can't use this trick for some types of
+ * file handle at all - for some reason Windows restricts use of
+ * OVERLAPPED to files which were opened with the overlapped flag -
+ * and so we must use threads for those. This being the case, it's
+ * simplest just to use threads for everything rather than trying
+ * to keep track of multiple completely separate mechanisms.)
  */
 
 #include <assert.h>
@@ -75,6 +86,11 @@ struct handle_input {
     void *privdata;                   /* for client to remember who they are */
 
     /*
+     * Data set at initialisation and then read-only.
+     */
+    int flags;
+
+    /*
      * Data set by the input thread before signalling ev_to_main,
      * and read by the main thread after receiving that signal.
      */
@@ -95,13 +111,41 @@ struct handle_input {
 static DWORD WINAPI handle_input_threadfunc(void *param)
 {
     struct handle_input *ctx = (struct handle_input *) param;
+    OVERLAPPED ovl, *povl;
+    HANDLE oev;
+    int readlen;
+
+    if (ctx->flags & HANDLE_FLAG_OVERLAPPED) {
+       povl = &ovl;
+       oev = CreateEvent(NULL, TRUE, FALSE, NULL);
+    } else {
+       povl = NULL;
+    }
+
+    if (ctx->flags & HANDLE_FLAG_UNITBUFFER)
+       readlen = 1;
+    else
+       readlen = sizeof(ctx->buffer);
 
     while (1) {
-       ctx->readret = ReadFile(ctx->h, ctx->buffer, sizeof(ctx->buffer),
-                               &ctx->len, NULL);
+       if (povl) {
+           memset(povl, 0, sizeof(OVERLAPPED));
+           povl->hEvent = oev;
+       }
+       ctx->readret = ReadFile(ctx->h, ctx->buffer, readlen,
+                               &ctx->len, povl);
+       if (povl && !ctx->readret && GetLastError() == ERROR_IO_PENDING) {
+           WaitForSingleObject(povl->hEvent, INFINITE);
+           ctx->readret = GetOverlappedResult(ctx->h, povl, &ctx->len, FALSE);
+       }
+
        if (!ctx->readret)
            ctx->len = 0;
 
+       if (ctx->readret && ctx->len == 0 &&
+           (ctx->flags & HANDLE_FLAG_IGNOREEOF))
+           continue;
+
        SetEvent(ctx->ev_to_main);
 
        if (!ctx->len)
@@ -112,6 +156,9 @@ static DWORD WINAPI handle_input_threadfunc(void *param)
            break;                     /* main thread told us to shut down */
     }
 
+    if (povl)
+       CloseHandle(oev);
+
     return 0;
 }
 
@@ -122,7 +169,8 @@ static DWORD WINAPI handle_input_threadfunc(void *param)
  */
 static void handle_throttle(struct handle_input *ctx, int backlog)
 {
-    assert(!ctx->defunct);
+    if (ctx->defunct)
+       return;
 
     /*
      * If there's a read operation already in progress, do nothing:
@@ -163,6 +211,11 @@ struct handle_output {
     void *privdata;                   /* for client to remember who they are */
 
     /*
+     * Data set at initialisation and then read-only.
+     */
+    int flags;
+
+    /*
      * Data set by the main thread before signalling ev_from_main,
      * and read by the input thread after receiving that signal.
      */
@@ -191,6 +244,12 @@ struct handle_output {
 static DWORD WINAPI handle_output_threadfunc(void *param)
 {
     struct handle_output *ctx = (struct handle_output *) param;
+    OVERLAPPED ovl, *povl;
+
+    if (ctx->flags & HANDLE_FLAG_OVERLAPPED)
+       povl = &ovl;
+    else
+       povl = NULL;
 
     while (1) {
        WaitForSingleObject(ctx->ev_from_main, INFINITE);
@@ -198,8 +257,14 @@ static DWORD WINAPI handle_output_threadfunc(void *param)
            SetEvent(ctx->ev_to_main);
            break;
        }
+       if (povl)
+           memset(povl, 0, sizeof(OVERLAPPED));
        ctx->writeret = WriteFile(ctx->h, ctx->buffer, ctx->len,
-                                 &ctx->lenwritten, NULL);
+                                 &ctx->lenwritten, povl);
+       if (povl && !ctx->writeret && GetLastError() == ERROR_IO_PENDING)
+           ctx->writeret = GetOverlappedResult(ctx->h, povl,
+                                               &ctx->lenwritten, TRUE);
+
        SetEvent(ctx->ev_to_main);
        if (!ctx->writeret)
            break;
@@ -264,9 +329,10 @@ static int handle_find_evtomain(void *av, void *bv)
 }
 
 struct handle *handle_input_new(HANDLE handle, handle_inputfn_t gotdata,
-                               void *privdata)
+                               void *privdata, int flags)
 {
     struct handle *h = snew(struct handle);
+    DWORD in_threadid; /* required for Win9x */
 
     h->output = FALSE;
     h->u.i.h = handle;
@@ -277,22 +343,24 @@ struct handle *handle_input_new(HANDLE handle, handle_inputfn_t gotdata,
     h->u.i.moribund = FALSE;
     h->u.i.done = FALSE;
     h->u.i.privdata = privdata;
+    h->u.i.flags = flags;
 
     if (!handles_by_evtomain)
        handles_by_evtomain = newtree234(handle_cmp_evtomain);
     add234(handles_by_evtomain, h);
 
     CreateThread(NULL, 0, handle_input_threadfunc,
-                &h->u.i, 0, NULL);
+                &h->u.i, 0, &in_threadid);
     h->u.i.busy = TRUE;
 
     return h;
 }
 
 struct handle *handle_output_new(HANDLE handle, handle_outputfn_t sentdata,
-                                void *privdata)
+                                void *privdata, int flags)
 {
     struct handle *h = snew(struct handle);
+    DWORD out_threadid; /* required for Win9x */
 
     h->output = TRUE;
     h->u.o.h = handle;
@@ -305,13 +373,14 @@ struct handle *handle_output_new(HANDLE handle, handle_outputfn_t sentdata,
     h->u.o.privdata = privdata;
     bufchain_init(&h->u.o.queued_data);
     h->u.o.sentdata = sentdata;
+    h->u.o.flags = flags;
 
     if (!handles_by_evtomain)
        handles_by_evtomain = newtree234(handle_cmp_evtomain);
     add234(handles_by_evtomain, h);
 
     CreateThread(NULL, 0, handle_output_threadfunc,
-                &h->u.i, 0, NULL);
+                &h->u.i, 0, &out_threadid);
 
     return h;
 }