Changeset 47537 in vbox for trunk/src/VBox

Timestamp:

Aug 5, 2013 10:00:02 AM (11 years ago)

Author:

vboxsync

Message:

HostDrivers/Support: Introduce reference counting for the session object to keep it alive while there is someone accessing it, fixes R0 assertion on OS X when killing the VM process

Location:

trunk/src/VBox/HostDrivers/Support

Files:

• SUPDrv.c (modified) (5 diffs)
• SUPDrvInternal.h (modified) (2 diffs)
• darwin/SUPDrv-darwin.cpp (modified) (4 diffs)
• freebsd/SUPDrv-freebsd.c (modified) (1 diff)
• linux/SUPDrv-linux.c (modified) (1 diff)
• os2/SUPDrv-os2.cpp (modified) (5 diffs)
• solaris/SUPDrv-solaris.c (modified) (1 diff)
• win/SUPDrv-win.cpp (modified) (1 diff)

trunk/src/VBox/HostDrivers/Support/SUPDrv.c

@@ -697 +697 @@
                 pSession->u32Cookie         = BIRD_INV;
                 pSession->fUnrestricted     = fUnrestricted;
+                pSession->cRefs             = 1;
                 /*pSession->pLdrUsage         = NULL;
                 pSession->pVM               = NULL;
@@ -742 +743 @@
 
 /**
- * Shared code for cleaning up a session.
+ * Shared code for cleaning up a session (but not quite freeing it).
+ *
+ * This is primarily intended for MAC OS X where we have to clean up the memory
+ * stuff before the file handle is closed.
  *
  * @param   pDevExt     Device extension.
@@ -748 +752 @@
  *                      This data will be freed by this routine.
  */
-void VBOXCALL supdrvCloseSession(PSUPDRVDEVEXT pDevExt, PSUPDRVSESSION pSession)
-{
-    VBOXDRV_SESSION_CLOSE(pSession);
-
-    /*
-     * Cleanup the session first.
-     */
-    supdrvCleanupSession(pDevExt, pSession);
-
-    /*
-     * Free the rest of the session stuff.
-     */
-    RTSpinlockDestroy(pSession->Spinlock);
-    pSession->Spinlock = NIL_RTSPINLOCK;
-    pSession->pDevExt = NULL;
-    RTMemFree(pSession);
-    LogFlow(("supdrvCloseSession: returns\n"));
-}
-
-
-/**
- * Shared code for cleaning up a session (but not quite freeing it).
- *
- * This is primarily intended for MAC OS X where we have to clean up the memory
- * stuff before the file handle is closed.
- *
- * @param   pDevExt     Device extension.
- * @param   pSession    Session data.
- *                      This data will be freed by this routine.
- */
-void VBOXCALL supdrvCleanupSession(PSUPDRVDEVEXT pDevExt, PSUPDRVSESSION pSession)
+static void supdrvCleanupSession(PSUPDRVDEVEXT pDevExt, PSUPDRVSESSION pSession)
 {
     int                 rc;
@@ -985 +959 @@
 }
 
+
+/**
+ * Shared code for cleaning up a session.
+ *
+ * @param   pDevExt     Device extension.
+ * @param   pSession    Session data.
+ *                      This data will be freed by this routine.
+ */
+static void supdrvCloseSession(PSUPDRVDEVEXT pDevExt, PSUPDRVSESSION pSession)
+{
+    VBOXDRV_SESSION_CLOSE(pSession);
+
+    /*
+     * Cleanup the session first.
+     */
+    supdrvCleanupSession(pDevExt, pSession);
+
+    /*
+     * Free the rest of the session stuff.
+     */
+    RTSpinlockDestroy(pSession->Spinlock);
+    pSession->Spinlock = NIL_RTSPINLOCK;
+    pSession->pDevExt = NULL;
+    RTMemFree(pSession);
+    LogFlow(("supdrvCloseSession: returns\n"));
+}
+
+
+/**
+ * Retain a session to make sure it doesn't go away while it is in use.
+ *
+ * @returns New reference count on success, UINT32_MAX on failure.
+ * @param   pSession    Session data.
+ */
+uint32_t VBOXCALL supdrvSessionRetain(PSUPDRVSESSION pSession)
+{
+    AssertPtrReturn(pSession, UINT32_MAX);
+    AssertReturn(SUP_IS_SESSION_VALID(pSession), UINT32_MAX);
+
+    uint32_t cRefs = ASMAtomicIncU32(&pSession->cRefs);
+    AssertMsg(cRefs > 1 && cRefs < _1M, ("%#x %p\n", cRefs, pSession));
+    return cRefs;
+}
+
+/**
+ * Releases a given session.
+ *
+ * @returns New reference count on success (0 if closed), UINT32_MAX on failure.
+ * @param   pSession    Session data.
+ */
+uint32_t VBOXCALL supdrvSessionRelease(PSUPDRVSESSION pSession)
+{
+    AssertPtrReturn(pSession, UINT32_MAX);
+    AssertReturn(SUP_IS_SESSION_VALID(pSession), UINT32_MAX);
+
+    uint32_t cRefs = ASMAtomicDecU32(&pSession->cRefs);
+    AssertMsg(cRefs < _1M, ("%#x %p\n", cRefs, pSession));
+    if (cRefs == 0)
+        supdrvCloseSession(pSession->pDevExt, pSession);
+    return cRefs;
+}
 
 /**
@@ -2121 +2156 @@
             REQ_CHECK_IDC_SIZE(SUPDRV_IDC_REQ_DISCONNECT, sizeof(*pReqHdr));
 
-            supdrvCloseSession(pDevExt, pSession);
+            supdrvSessionRelease(pSession);
             return pReqHdr->rc = VINF_SUCCESS;
         }

trunk/src/VBox/HostDrivers/Support/SUPDrvInternal.h

@@ -419 +419 @@
     /** Set if is an unrestricted session, clear if restricted. */
     bool                            fUnrestricted;
+    /* Reference counter. */
+    uint32_t volatile               cRefs;
 
     /** The VM associated with the session. */
@@ -693 +695 @@
 void VBOXCALL   supdrvDeleteDevExt(PSUPDRVDEVEXT pDevExt);
 int  VBOXCALL   supdrvCreateSession(PSUPDRVDEVEXT pDevExt, bool fUser, bool fUnrestricted,  PSUPDRVSESSION *ppSession);
-void VBOXCALL   supdrvCloseSession(PSUPDRVDEVEXT pDevExt, PSUPDRVSESSION pSession);
-void VBOXCALL   supdrvCleanupSession(PSUPDRVDEVEXT pDevExt, PSUPDRVSESSION pSession);
+uint32_t VBOXCALL supdrvSessionRetain(PSUPDRVSESSION pSession);
+uint32_t VBOXCALL supdrvSessionRelease(PSUPDRVSESSION pSession);
 
 int  VBOXCALL   supdrvTracerInit(PSUPDRVDEVEXT pDevExt);

trunk/src/VBox/HostDrivers/Support/darwin/SUPDrv-darwin.cpp

@@ -524 +524 @@
     while (pSession && pSession->Process != Process && pSession->fUnrestricted == fUnrestricted && pSession->fOpened)
         pSession = pSession->pNextHash;
+
+    if (RT_LIKELY(pSession))
+        supdrvSessionRetain(pSession);
     RTSpinlockReleaseNoInts(g_Spinlock);
     if (!pSession)
@@ -536 +539 @@
      * the session and iCmd, and only returns a VBox status code.
      */
+    int rc;
     if (   (    iCmd == SUP_IOCTL_FAST_DO_RAW_RUN
             ||  iCmd == SUP_IOCTL_FAST_DO_HM_RUN
             ||  iCmd == SUP_IOCTL_FAST_DO_NOP)
         && fUnrestricted)
-        return supdrvIOCtlFast(iCmd, *(uint32_t *)pData, &g_DevExt, pSession);
-    return VBoxDrvDarwinIOCtlSlow(pSession, iCmd, pData, pProcess);
+        rc = supdrvIOCtlFast(iCmd, *(uint32_t *)pData, &g_DevExt, pSession);
+    else
+        rc = VBoxDrvDarwinIOCtlSlow(pSession, iCmd, pData, pProcess);
+
+    supdrvSessionRelease(pSession);
+    return rc;
 }
 
@@ -1155 +1163 @@
 
                 LogFlow(("org_virtualbox_SupDrvClient::start: already got a session for this process (%p)\n", pCur));
-                supdrvCloseSession(&g_DevExt, m_pSession);
+                supdrvSessionRelease(m_pSession);
             }
 
@@ -1231 +1239 @@
      * Close the session.
      */
-    supdrvCloseSession(&g_DevExt, pSession);
+    supdrvSessionRelease(pSession);
 }
 

trunk/src/VBox/HostDrivers/Support/freebsd/SUPDrv-freebsd.c

@@ -292 +292 @@
      * Close the session.
      */
-    supdrvCloseSession(&g_VBoxDrvFreeBSDDevExt, pSession);
+    supdrvSessionRelease(&g_VBoxDrvFreeBSDDevExt, pSession);
     ASMAtomicDecU32(&g_cUsers);
 }

trunk/src/VBox/HostDrivers/Support/linux/SUPDrv-linux.c

@@ -550 +550 @@
     Log(("VBoxDrvLinuxClose: pFilp=%p pSession=%p pid=%d/%d %s\n",
          pFilp, pFilp->private_data, RTProcSelf(), current->pid, current->comm));
-    supdrvCloseSession(&g_DevExt, (PSUPDRVSESSION)pFilp->private_data);
+    supdrvSessionRelease((PSUPDRVSESSION)pFilp->private_data);
     pFilp->private_data = NULL;
     return 0;

trunk/src/VBox/HostDrivers/Support/os2/SUPDrv-os2.cpp

@@ -222 +222 @@
      * Close the session.
      */
-    supdrvCloseSession(&g_DevExt, pSession);
+    supdrvSessionRelease(pSession);
     return 0;
 }
@@ -244 +244 @@
                &&   (   pSession->sfn != sfn
                      || pSession->Process != Process));
+
+        if (RT_LIKELY(pSession))
+            supdrvSessionRetain(pSession);
     }
     RTSpinlockReleaseNoInts(g_Spinlock);
@@ -256 +259 @@
      */
     supdrvIOCtlFast(iFunction, 0, &g_DevExt, pSession);
+    supdrvSessionRelease(pSession);
     return 0;
 }
@@ -277 +281 @@
                &&   (   pSession->sfn != sfn
                      || pSession->Process != Process));
+
+        if (RT_LIKELY(pSession))
+            supdrvSessionRetain(pSession);
     }
     RTSpinlockReleaseNoInts(g_Spinlock);
@@ -346 +353 @@
          */
         int rc2 = KernVMUnlock(&Lock);
-        AssertMsg(!rc2, ("rc2=%d\n", rc2)); NOREF(rc2);
-
-        Log2(("VBoxDrvIOCtl: returns %d\n", rc));
-        return rc;
-    }
-    return VERR_NOT_SUPPORTED;
+        AssertMsg(!rc2, ("rc2=%d\n", rc2)); NOREF(rc2);s
+    }
+    else
+        rc = VERR_NOT_SUPPORTED;
+
+    supdrvSessionRelease(pSession);
+    Log2(("VBoxDrvIOCtl: returns %d\n", rc));
+    return rc;
 }
 

trunk/src/VBox/HostDrivers/Support/solaris/SUPDrv-solaris.c

@@ -623 +623 @@
      * Close the session.
      */
-    supdrvCloseSession(&g_DevExt, pSession);
+    supdrvSessionRelease(pSession);
     return 0;
 }

trunk/src/VBox/HostDrivers/Support/win/SUPDrv-win.cpp

@@ -386 +386 @@
     if (pSession)
     {
-        supdrvCloseSession(pDevExt, (PSUPDRVSESSION)pFileObj->FsContext);
+        supdrvSessionRelease((PSUPDRVSESSION)pFileObj->FsContext);
         pFileObj->FsContext = NULL;
     }