From: VMware, Inc <>
Date: Tue, 29 Mar 2011 20:09:08 +0000 (-0700)
Subject: Fixing deadlock that occurs when cancelling unarchiving.
X-Git-Tag: 2011.03.28-387002~47
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6be0ef2a6d5e8eb3624dd77478bb0be921de506e;p=thirdparty%2Fopen-vm-tools.git

Fixing deadlock that occurs when cancelling unarchiving.

When MacOS mutex is acquired recursively by the same thread a deadlock occurs.
Thus it is unsafe to hold a mutex that is maintained by HGFS while invoking a
system call that may result in recursively calling a vnode method which may
in turn try to acquire the same mutex.
Particulary vnode hash table mutex should be released before invoking a
HgfsFreeFile function that invokes vnode_rele which may in turn call
inavtivate or reclaim vnode methods.
Current implementation still calls vnode_get while holding the mutex. It is OK,
since vnode_get just increases reference count and it never result in
recursive call into HGFS.

Signed-off-by: Marcelo Vanzin <mvanzin@vmware.com>
---

diff --git a/open-vm-tools/modules/freebsd/vmhgfs/state.c b/open-vm-tools/modules/freebsd/vmhgfs/state.c
index f620c4b19..ca4b75a8b 100644
--- a/open-vm-tools/modules/freebsd/vmhgfs/state.c
+++ b/open-vm-tools/modules/freebsd/vmhgfs/state.c
@@ -101,7 +101,6 @@ static void HgfsFreeFile(HgfsFile *fp);
 
 /* Adding/finding/removing file state from hash table */
 static void HgfsAddFile(HgfsFile *fp, HgfsFileHashTable *htp);
-static void HgfsRemoveFile(HgfsFile *fp, HgfsFileHashTable *htp);
 static HgfsFile *HgfsFindFile(const char *fileName, HgfsFileHashTable *htp);
 
 /* Other utility functions */
@@ -680,7 +679,7 @@ HgfsLookupExistingVnode(const char* fileName,
          } else {
             /* vnode exists but unusable, remove HGFS context assosiated with it. */
             DEBUG(VM_DEBUG_FAIL, "Removing HgfsFile assosiated with an unusable vnode\n");
-            HgfsRemoveFile(existingFp, htp);
+            DblLnkLst_Unlink1(&existingFp->listNode);
             err = ENOENT;
          }
       }
@@ -1097,15 +1096,14 @@ HgfsReleaseFile(HgfsFile *fp,           // IN: File to release
    ASSERT(fp);
    ASSERT(htp);
 
-   os_mutex_lock(htp->mutex);
-
    DEBUG(VM_DEBUG_INFO, "HgfsReleaseFile: freeing HgfsFile for %s.\n",
          fp->fileName);
    /* Take this file off its list */
+   os_mutex_lock(htp->mutex);
    DblLnkLst_Unlink1(&fp->listNode);
-   HgfsFreeFile(fp);
-
    os_mutex_unlock(htp->mutex);
+
+   HgfsFreeFile(fp);
 }
 
 
@@ -1382,43 +1380,6 @@ HgfsFindFile(const char *fileName,      // IN: Filename to look for
 }
 
 
-/*
- *----------------------------------------------------------------------------
- *
- * HgfsRemoveFile --
- *
- *      Removes file from the hash table.
- *
- *      Note that unlike the other two hash functions, this one performs its own
- *      locking since the removal doesn't need to be atomic with other
- *      operations.  (This could change in the future if the functions that use
- *      this one are reorganized.)
- *
- * Results:
- *      Returns 0 on success and a non-zero error code on failure.
- *
- * Side effects:
- *      None.
- *
- *----------------------------------------------------------------------------
- */
-
-static void
-HgfsRemoveFile(HgfsFile *fp,            // IN: File to remove
-               HgfsFileHashTable *htp)  // IN: Hash table to remove from
-{
-   ASSERT(fp);
-   ASSERT(htp);
-
-   os_mutex_lock(htp->mutex);
-
-   /* Take this file off its list */
-   DblLnkLst_Unlink1(&fp->listNode);
-
-   os_mutex_unlock(htp->mutex);
-}
-
-
 /* Other utility functions */