Changeset 57600 in vbox for trunk/src/VBox/Devices

Timestamp:

Sep 2, 2015 4:42:05 PM (10 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

102461

Message:

NAT: don't deadlock if we try to activate a NAT network while the VM is not running

File:

• trunk/src/VBox/Devices/Network/DrvNAT.cpp (modified) (7 diffs)

trunk/src/VBox/Devices/Network/DrvNAT.cpp

@@ -575 +575 @@
         PDMDrvHlpFTSetCheckpoint(pThis->pDrvIns, FTMCHECKPOINTTYPE_NETWORK);
 
-
-        RTREQQUEUE hQueue = pThis->hSlirpReqQueue;
-
-        rc = RTReqQueueCallEx(hQueue, NULL /*ppReq*/, 0 /*cMillies*/, RTREQFLAGS_VOID | RTREQFLAGS_NO_WAIT,
+        rc = RTReqQueueCallEx(pThis->hSlirpReqQueue, NULL /*ppReq*/, 0 /*cMillies*/,
+                              RTREQFLAGS_VOID | RTREQFLAGS_NO_WAIT,
                               (PFNRT)drvNATSendWorker, 2, pThis, pSgBuf);
         if (RT_SUCCESS(rc))
@@ -667 +665 @@
     LogFlow(("drvNATNetworkUp_NotifyLinkChanged: enmLinkState=%d\n", enmLinkState));
 
-    /* Don't queue new requests when the NAT thread is about to stop.
-     * But the VM could also be paused. So memorize the desired state. */
+    /* Don't queue new requests if the NAT thread is not running (e.g. paused,
+     * stopping), otherwise we would deadlock. Memorize the change. */
     if (pThis->pSlirpThread->enmState != PDMTHREADSTATE_RUNNING)
     {
@@ -678 +676 @@
     int rc = RTReqQueueCallEx(pThis->hSlirpReqQueue, &pReq, 0 /*cMillies*/, RTREQFLAGS_VOID,
                               (PFNRT)drvNATNotifyLinkChangedWorker, 2, pThis, enmLinkState);
-    if (RT_LIKELY(rc == VERR_TIMEOUT))
+    if (rc == VERR_TIMEOUT)
     {
         drvNATNotifyNATThread(pThis, "drvNATNetworkUp_NotifyLinkChanged");
@@ -714 +712 @@
 }
 
-static DECLCALLBACK(int) drvNATNetworkNatConfig_RedirectRuleCommand(PPDMINETWORKNATCONFIG pInterface, bool fRemove,
-                                                             bool fUdp, const char *pHostIp,
-                                                             uint16_t u16HostPort, const char *pGuestIp, uint16_t u16GuestPort)
+static DECLCALLBACK(int) drvNATNetworkNatConfigRedirect(PPDMINETWORKNATCONFIG pInterface, bool fRemove,
+                                                        bool fUdp, const char *pHostIp, uint16_t u16HostPort,
+                                                        const char *pGuestIp, uint16_t u16GuestPort)
 {
     LogFlowFunc(("fRemove=%d, fUdp=%d, pHostIp=%s, u16HostPort=%u, pGuestIp=%s, u16GuestPort=%u\n",
-                 RT_BOOL(fRemove), RT_BOOL(fUdp), pHostIp, u16HostPort, pGuestIp,
-                 u16GuestPort));
+                 RT_BOOL(fRemove), RT_BOOL(fUdp), pHostIp, u16HostPort, pGuestIp, u16GuestPort));
     PDRVNAT pThis = RT_FROM_MEMBER(pInterface, DRVNAT, INetworkNATCfg);
-    PRTREQ pReq;
-    int rc = RTReqQueueCallEx(pThis->hSlirpReqQueue, &pReq, 0 /*cMillies*/, RTREQFLAGS_VOID,
+    /* Execute the command directly if the VM is not running. */
+    int rc;
+    if (pThis->pSlirpThread->enmState != PDMTHREADSTATE_RUNNING)
+    {
+        drvNATNotifyApplyPortForwardCommand(pThis, fRemove, fUdp, pHostIp,
+                                           u16HostPort, pGuestIp,u16GuestPort);
+        rc = VINF_SUCCESS;
+    }
+    else
+    {
+        PRTREQ pReq;
+        rc = RTReqQueueCallEx(pThis->hSlirpReqQueue, &pReq, 0 /*cMillies*/, RTREQFLAGS_VOID,
                               (PFNRT)drvNATNotifyApplyPortForwardCommand, 7, pThis, fRemove,
                               fUdp, pHostIp, u16HostPort, pGuestIp, u16GuestPort);
-    if (RT_LIKELY(rc == VERR_TIMEOUT))
-    {
-        drvNATNotifyNATThread(pThis, "drvNATNetworkNatConfig_RedirectRuleCommand");
-        rc = RTReqWait(pReq, RT_INDEFINITE_WAIT);
-        AssertRC(rc);
-    }
-    else
-        AssertRC(rc);
-
-    RTReqRelease(pReq);
-    port_forwarding_done:
+        if (rc == VERR_TIMEOUT)
+        {
+            drvNATNotifyNATThread(pThis, "drvNATNetworkNatConfigRedirect");
+            rc = RTReqWait(pReq, RT_INDEFINITE_WAIT);
+            AssertRC(rc);
+        }
+        else
+            AssertRC(rc);
+
+        RTReqRelease(pReq);
+    }
     return rc;
 }
@@ -1143 +1150 @@
     switch (strategy)
     {
-
         case VBOX_NAT_DNS_DNSPROXY:
-            {
-                /**
-                 * XXX: Here or in _strategy_selector we should deal with network change
-                 * in "network change" scenario domain name change we have to update guest lease
-                 * forcibly.
-                 * Note at that built-in dhcp also updates DNS information on NAT thread.
-                 */
-                /**
-                 * It's unsafe to to do it directly on non-NAT thread
-                 * so we schedule the worker and kick the NAT thread.
-                 */
-                RTREQQUEUE hQueue = pThis->hSlirpReqQueue;
-
-                int rc = RTReqQueueCallEx(hQueue, NULL /*ppReq*/, 0 /*cMillies*/,
-                                          RTREQFLAGS_VOID | RTREQFLAGS_NO_WAIT,
-                                          (PFNRT)drvNATReinitializeHostNameResolving, 1, pThis);
-                if (RT_SUCCESS(rc))
-                    drvNATNotifyNATThread(pThis, "drvNATUpdateDNS");
-
-                return;
-            }
+        {
+            /**
+             * XXX: Here or in _strategy_selector we should deal with network change
+             * in "network change" scenario domain name change we have to update guest lease
+             * forcibly.
+             * Note at that built-in dhcp also updates DNS information on NAT thread.
+             */
+            /**
+             * It's unsafe to to do it directly on non-NAT thread
+             * so we schedule the worker and kick the NAT thread.
+             */
+            int rc = RTReqQueueCallEx(pThis->hSlirpReqQueue, NULL /*ppReq*/, 0 /*cMillies*/,
+                                      RTREQFLAGS_VOID | RTREQFLAGS_NO_WAIT,
+                                      (PFNRT)drvNATReinitializeHostNameResolving, 1, pThis);
+            if (RT_SUCCESS(rc))
+                drvNATNotifyNATThread(pThis, "drvNATUpdateDNS");
+
+            return;
+        }
 
         case VBOX_NAT_DNS_EXTERNAL:
@@ -1172 +1176 @@
              * Disconnect the guest from the network temporarily to let it pick up the changes.
              */
-
             if (fFlapLink)
                 pThis->pIAboveConfig->pfnSetLinkState(pThis->pIAboveConfig,
@@ -1421 +1424 @@
 
     /* NAT engine configuration */
-    pThis->INetworkNATCfg.pfnRedirectRuleCommand = drvNATNetworkNatConfig_RedirectRuleCommand;
+    pThis->INetworkNATCfg.pfnRedirectRuleCommand = drvNATNetworkNatConfigRedirect;
 #if HAVE_NOTIFICATION_FOR_DNS_UPDATE && !defined(RT_OS_DARWIN)
     /*