From: Owen Smith <owen.smith@citrix.com>
Date: Tue, 29 Aug 2017 10:07:48 +0000 (+0100)
Subject: Ensure handles are closed when pipe disconnects
X-Git-Tag: 9.0.0-rc1~38
X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=5cbe28d67e7ed0cd3dc7615c14ef36efd808306c;p=pvdrivers%2Fwin%2Fxencons.git

Ensure handles are closed when pipe disconnects

Fixes leaks where the pipe handles were not closed, preventing new
connections. Fixes the MONITOR_PIPE context leaking when its thread
is stopped.

Signed-off-by: Owen Smith <owen.smith@citrix.com>

Since error gotos are introduced, have them do the cleanup. The scope of
Pipe and Instance needs to be widened but it keeps the code neater overall.
Also removed the unnecessary newlines in the log messages.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---

diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 3e535e4..e54f47d 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -469,6 +469,10 @@ PipeThread(
 
     CloseHandle(Overlapped.hEvent);
 
+    CloseHandle(Pipe->Pipe);
+    CloseHandle(Pipe->Thread);
+    free(Pipe);
+
     Log("<====");
 
     return 0;
@@ -494,7 +498,9 @@ ServerThread(
     PMONITOR_CONTEXT    Context = &MonitorContext;
     OVERLAPPED          Overlapped;
     HANDLE              Handle[2];
+    HANDLE              Pipe;
     DWORD               Object;
+    PMONITOR_PIPE       Instance;
     HRESULT             Error;
 
     UNREFERENCED_PARAMETER(Argument);
@@ -513,9 +519,6 @@ ServerThread(
     Handle[1] = Overlapped.hEvent;
 
     for (;;) {
-        HANDLE          Pipe;
-        PMONITOR_PIPE   Instance;
-
         Pipe = CreateNamedPipe(PIPE_NAME,
                                PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED,
                                PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE,
@@ -525,7 +528,7 @@ ServerThread(
                                0,
                                NULL);
         if (Pipe == INVALID_HANDLE_VALUE)
-            break;
+            goto fail2;
 
         (VOID) ConnectNamedPipe(Pipe,
                                 &Overlapped);
@@ -534,16 +537,16 @@ ServerThread(
                                         Handle,
                                         FALSE,
                                         INFINITE);
-        if (Object == WAIT_OBJECT_0)
+        if (Object == WAIT_OBJECT_0) {
+            CloseHandle(Pipe);
             break;
+        }
 
         ResetEvent(Overlapped.hEvent);
 
         Instance = (PMONITOR_PIPE)malloc(sizeof(MONITOR_PIPE));
-        if (Instance == NULL) {
-            CloseHandle(Pipe);
-            break;
-        }
+        if (Instance == NULL)
+            goto fail3;
 
         __InitializeListHead(&Instance->ListEntry);
         Instance->Pipe = Pipe;
@@ -554,11 +557,8 @@ ServerThread(
                                         Instance,
                                         0,
                                         NULL);
-        if (Instance->Thread == INVALID_HANDLE_VALUE) {
-            free(Instance);
-            CloseHandle(Pipe);
-            break;
-        }
+        if (Instance->Thread == INVALID_HANDLE_VALUE)
+            goto fail4;
     }
 
     CloseHandle(Overlapped.hEvent);
@@ -567,6 +567,19 @@ ServerThread(
 
     return 0;
 
+fail4:
+    Log("fail4");
+
+    free(Instance);
+
+fail3:
+    Log("fail3");
+
+    CloseHandle(Pipe);
+
+fail2:
+    Log("fail2");
+
 fail1:
     Error = GetLastError();