From: Oliver Kurth <okurth@vmware.com>
Date: Fri, 15 Sep 2017 18:23:40 +0000 (-0700)
Subject: Hgfs Server: Fix a memory leak of the transport connection
X-Git-Tag: stable-10.2.0~201
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e146ddb85e0f620faff25decc716ad406b4486e8;p=thirdparty%2Fopen-vm-tools.git

Hgfs Server: Fix a memory leak of the transport connection

This change fixes a memory leak of the transport connection object
which was not being released on the final reference removal when
the HGFS server is being torn down.

This separates the initialization and destruction of the transport
object into their own transport init and exit functions.  Added the
lock destruct and free call of the transport object to the exit
function that were previously missing.  Corrected or added some
missing logging for debugging.

Additionally, fixed the VMCI interface to correct its tear down
order of functions which relied on the transport not being deleted.
This involved moving the draining of out of band HGFS server requests
or replies to the guest from being called after the HGFS server
transport has been destroyed.  Now it is moved to the correct place,
where the sequence is now:
- disconnect the HGFS server (i.e., stop generating any new request/replies out of band,
- drain the existing out of band HGFS requests/replies
- close the VMCI transport and shared memory callbacks
- close and teardown the HGFS server session and transport
- teardown the VMCI channel itself
---

diff --git a/open-vm-tools/lib/hgfsServer/hgfsServer.c b/open-vm-tools/lib/hgfsServer/hgfsServer.c
index 95cbb220c..ae9b29f90 100644
--- a/open-vm-tools/lib/hgfsServer/hgfsServer.c
+++ b/open-vm-tools/lib/hgfsServer/hgfsServer.c
@@ -291,6 +291,12 @@ HgfsServerTransportRemoveSessionFromList(HgfsTransportSessionInfo *transportSess
 static HgfsSessionInfo *
 HgfsServerTransportGetSessionInfo(HgfsTransportSessionInfo *transportSession,
                                   uint64 sessionId);
+static HgfsTransportSessionInfo *
+HgfsServerTransportInit(void *transportData,
+                        HgfsServerChannelCallbacks *channelCbTable,
+                        HgfsServerChannelData *channelCapabilities);
+static void
+HgfsServerTransportExit(HgfsTransportSessionInfo *transportSession);
 
 /* Local functions. */
 static void HgfsInvalidateSessionObjects(DblLnkLst_Links *shares,
@@ -483,17 +489,7 @@ HgfsServerTransportSessionPut(HgfsTransportSessionInfo *transportSession)   // I
 {
    ASSERT(transportSession);
    if (Atomic_ReadDec32(&transportSession->refCount) == 1) {
-      DblLnkLst_Links *curr, *next;
-
-      MXUser_AcquireExclLock(transportSession->sessionArrayLock);
-
-      DblLnkLst_ForEachSafe(curr, next,  &transportSession->sessionArray) {
-         HgfsSessionInfo *session = DblLnkLst_Container(curr, HgfsSessionInfo, links);
-         HgfsServerTransportRemoveSessionFromList(transportSession, session);
-         HgfsServerSessionPut(session);
-      }
-
-      MXUser_ReleaseExclLock(transportSession->sessionArrayLock);
+      HgfsServerTransportExit(transportSession);
    }
 }
 
@@ -4244,12 +4240,42 @@ HgfsServerSessionConnect(void *transportData,                         // IN: tra
                          HgfsServerChannelData *channelCapabilities,  // IN: channel capabilities
                          void **transportSessionData)                 // OUT: server session context
 {
-   HgfsTransportSessionInfo *transportSession;
-
    ASSERT(transportSessionData);
 
    LOG(4, ("%s: initting.\n", __FUNCTION__));
 
+   *transportSessionData = HgfsServerTransportInit(transportData,
+                                                   channelCbTable,
+                                                   channelCapabilities);
+   return TRUE;
+}
+
+
+/*
+ *----------------------------------------------------------------------------
+ *
+ * HgfsServerTransportInit --
+ *
+ *      Init a transport session.
+ *
+ *      Allocated and initialize the transport session.
+ *
+ * Results:
+ *      The initialized transport session object.
+ *
+ * Side effects:
+ *      None.
+ *
+ *----------------------------------------------------------------------------
+ */
+
+static HgfsTransportSessionInfo *
+HgfsServerTransportInit(void *transportData,                         // IN: transport session context
+                        HgfsServerChannelCallbacks *channelCbTable,  // IN: Channel callbacks
+                        HgfsServerChannelData *channelCapabilities)  // IN: channel capabilities
+{
+   HgfsTransportSessionInfo *transportSession;
+
    transportSession = Util_SafeCalloc(1, sizeof *transportSession);
    transportSession->transportData = transportData;
    transportSession->channelCbTable = channelCbTable;
@@ -4270,9 +4296,43 @@ HgfsServerSessionConnect(void *transportData,                         // IN: tra
 
    /* Give our session a reference to hold while we are open. */
    HgfsServerTransportSessionGet(transportSession);
+   return transportSession;
+}
 
-   *transportSessionData = transportSession;
-   return TRUE;
+
+/*
+ *----------------------------------------------------------------------------
+ *
+ * HgfsServerTransportExit --
+ *
+ *      Exit by destroying the transport session.
+ *
+ *      Free session info data if no reference.
+ *
+ * Results:
+ *      None.
+ *
+ * Side effects:
+ *      None.
+ *
+ *----------------------------------------------------------------------------
+ */
+
+static void
+HgfsServerTransportExit(HgfsTransportSessionInfo *transportSession)   // IN: transport session context
+{
+   DblLnkLst_Links *curr, *next;
+
+   ASSERT(Atomic_Read(&transportSession->refCount) == 0);
+
+   DblLnkLst_ForEachSafe(curr, next,  &transportSession->sessionArray) {
+      HgfsSessionInfo *session = DblLnkLst_Container(curr, HgfsSessionInfo, links);
+      HgfsServerTransportRemoveSessionFromList(transportSession, session);
+      HgfsServerSessionPut(session);
+   }
+
+   MXUser_DestroyExclLock(transportSession->sessionArrayLock);
+   free(transportSession);
 }
 
 
@@ -4515,11 +4575,14 @@ HgfsServerSessionClose(void *clientData)    // IN: session context
 {
    HgfsTransportSessionInfo *transportSession = clientData;
 
+   LOG(8, ("%s: entered\n", __FUNCTION__));
+
    ASSERT(transportSession);
    ASSERT(transportSession->state == HGFS_SESSION_STATE_CLOSED);
 
    /* Remove, typically, the last reference, will teardown everything. */
    HgfsServerTransportSessionPut(transportSession);
+   LOG(8, ("%s: exit\n", __FUNCTION__));
 }
 
 
@@ -4569,7 +4632,7 @@ HgfsServerExitSessionInternal(HgfsSessionInfo *session)    // IN: session contex
 
    MXUser_AcquireExclLock(session->nodeArrayLock);
 
-   Log("%s: exit session %p id %"FMT64"x\n", __FUNCTION__, session, session->sessionId);
+   Log("%s: teardown session %p id 0x%"FMT64"x\n", __FUNCTION__, session, session->sessionId);
 
    /* Recycle all nodes that are still in use, then destroy the node pool. */
    for (i = 0; i < session->numNodes; i++) {