Changeset 77323 in vbox for trunk/src/VBox/Runtime/r3

Timestamp:

Feb 14, 2019 9:20:55 PM (6 years ago)

Author:

vboxsync

Message:

Runtime/serialport-win.cpp: Don't keep dwEventMask on the stack for overlapped I/O as this may get written to after the method returned corrupting stack memory. Fixes crashes on Windows

File:

• trunk/src/VBox/Runtime/r3/win/serialport-win.cpp (modified) (5 diffs)

trunk/src/VBox/Runtime/r3/win/serialport-win.cpp

@@ -76 +76 @@
     /** Events currently waited for. */
     uint32_t            fEvtMask;
+    /** Event mask as received by WaitCommEvent(). */
+    DWORD               dwEventMask;
     /** Flag whether a write is currently pending. */
     bool                fWritePending;
+    /** Event query pending. */
+    bool                fEvtQueryPending;
     /** Bounce buffer for writes. */
     uint8_t            *pbBounceBuf;
@@ -247 +251 @@
 
 
+/**
+ * Processes the received Windows comm events and converts them to our format.
+ *
+ * @returns IPRT status code.
+ * @param   pThis               The pipe instance handle.
+ * @param   dwEventMask         Event mask received.
+ * @param   pfEvtsRecv          Where to store the converted events.
+ */
+static int rtSerialPortEvtFlagsProcess(PRTSERIALPORTINTERNAL pThis, DWORD dwEventMask, uint32_t *pfEvtsRecv)
+{
+    int rc = VINF_SUCCESS;
+
+    if (dwEventMask & EV_RXCHAR)
+        *pfEvtsRecv |= RTSERIALPORT_EVT_F_DATA_RX;
+    if (dwEventMask & EV_TXEMPTY)
+    {
+        if (pThis->fWritePending)
+        {
+            rc = rtSerialPortWriteCheckCompletion(pThis);
+            if (rc == VINF_SUCCESS)
+                *pfEvtsRecv |= RTSERIALPORT_EVT_F_DATA_TX;
+            else
+                rc = VINF_SUCCESS;
+        }
+        else
+            *pfEvtsRecv |= RTSERIALPORT_EVT_F_DATA_TX;
+    }
+    if (dwEventMask & EV_BREAK)
+        *pfEvtsRecv |= RTSERIALPORT_EVT_F_BREAK_DETECTED;
+    if (dwEventMask & (EV_CTS | EV_DSR | EV_RING | EV_RLSD))
+        *pfEvtsRecv |= RTSERIALPORT_EVT_F_STATUS_LINE_CHANGED;
+
+    return rc;
+}
+
+
+/**
+ * The internal comm event wait worker.
+ *
+ * @returns IPRT status code.
+ * @param   pThis               The pipe instance handle.
+ * @param   msTimeout           The timeout to wait for.
+ */
+static int rtSerialPortEvtWaitWorker(PRTSERIALPORTINTERNAL pThis, RTMSINTERVAL msTimeout)
+{
+    int rc = VINF_SUCCESS;
+    HANDLE ahWait[2];
+    ahWait[0] = pThis->hEvtDev;
+    ahWait[1] = pThis->hEvtIntr;
+
+    DWORD dwRet = WaitForMultipleObjects(2, ahWait, FALSE, msTimeout == RT_INDEFINITE_WAIT ? INFINITE : msTimeout);
+    if (dwRet == WAIT_TIMEOUT)
+        rc = VERR_TIMEOUT;
+    else if (dwRet == WAIT_FAILED)
+        rc = RTErrConvertFromWin32(GetLastError());
+    else if (dwRet == WAIT_OBJECT_0)
+        rc = VINF_SUCCESS;
+    else
+    {
+        Assert(dwRet == WAIT_OBJECT_0 + 1);
+        rc = VERR_INTERRUPTED;
+    }
+
+    return rc;
+}
+
+
 RTDECL(int)  RTSerialPortOpen(PRTSERIALPORT phSerialPort, const char *pszPortAddress, uint32_t fFlags)
 {
@@ -263 +334 @@
         pThis->fEvtMask         = 0;
         pThis->fWritePending    = false;
+        pThis->fEvtQueryPending = false;
         pThis->pbBounceBuf      = NULL;
         pThis->cbBounceBufUsed  = 0;
@@ -786 +858 @@
     int rc = VINF_SUCCESS;
     if (fEvtMask != pThis->fEvtMask)
+    {
         rc = rtSerialPortWinUpdateEvtMask(pThis, fEvtMask);
+        if (   RT_SUCCESS(rc)
+            && pThis->fEvtQueryPending)
+        {
+            /*
+             * Setting a new event mask lets the WaitCommEvent() call finish immediately,
+             * so clean up and process any events here.
+             */
+            rc = rtSerialPortEvtWaitWorker(pThis, 1);
+            AssertRC(rc);
+
+            if (pThis->dwEventMask != 0)
+            {
+                pThis->fEvtQueryPending = false;
+                return rtSerialPortEvtFlagsProcess(pThis, pThis->dwEventMask, pfEvtsRecv);
+            }
+        }
+    }
 
     /*
@@ -817 +907 @@
     if (RT_SUCCESS(rc))
     {
-        DWORD dwEventMask = 0;
-        HANDLE ahWait[2];
-        ahWait[0] = pThis->hEvtDev;
-        ahWait[1] = pThis->hEvtIntr;
-
-        RT_ZERO(pThis->OverlappedEvt);
-        pThis->OverlappedEvt.hEvent = pThis->hEvtDev;
-
-        if (!WaitCommEvent(pThis->hDev, &dwEventMask, &pThis->OverlappedEvt))
-        {
-            DWORD dwRet = GetLastError();
-            if (dwRet == ERROR_IO_PENDING)
-            {
-                dwRet = WaitForMultipleObjects(2, ahWait, FALSE, msTimeout == RT_INDEFINITE_WAIT ? INFINITE : msTimeout);
-                if (dwRet == WAIT_TIMEOUT)
-                    rc = VERR_TIMEOUT;
-                else if (dwRet == WAIT_FAILED)
+        /* Set up a new event wait if there is none pending. */
+        if (!pThis->fEvtQueryPending)
+        {
+            RT_ZERO(pThis->OverlappedEvt);
+            pThis->OverlappedEvt.hEvent = pThis->hEvtDev;
+
+            pThis->dwEventMask = 0;
+            pThis->fEvtQueryPending = true;
+            if (!WaitCommEvent(pThis->hDev, &pThis->dwEventMask, &pThis->OverlappedEvt))
+            {
+                DWORD dwRet = GetLastError();
+                if (dwRet == ERROR_IO_PENDING)
+                    rc = VINF_SUCCESS;
+                else
                     rc = RTErrConvertFromWin32(GetLastError());
-                else if (dwRet != WAIT_OBJECT_0)
-                    rc = VERR_INTERRUPTED;
             }
             else
-                rc = RTErrConvertFromWin32(dwRet);
-        }
+                pThis->fEvtQueryPending = false;
+        }
+
+        Assert(RT_FAILURE(rc) || pThis->fEvtQueryPending);
+
+        if (   RT_SUCCESS(rc)
+            || pThis->fEvtQueryPending)
+            rc = rtSerialPortEvtWaitWorker(pThis, msTimeout);
 
         if (RT_SUCCESS(rc))
         {
-            /* Check the event */
-            if (dwEventMask & EV_RXCHAR)
-                *pfEvtsRecv |= RTSERIALPORT_EVT_F_DATA_RX;
-            if (dwEventMask & EV_TXEMPTY)
-            {
-                if (pThis->fWritePending)
-                {
-                    rc = rtSerialPortWriteCheckCompletion(pThis);
-                    if (rc == VINF_SUCCESS)
-                        *pfEvtsRecv |= RTSERIALPORT_EVT_F_DATA_TX;
-                    else
-                        rc = VINF_SUCCESS;
-                }
-                else
-                    *pfEvtsRecv |= RTSERIALPORT_EVT_F_DATA_TX;
-            }
-            if (dwEventMask & EV_BREAK)
-                *pfEvtsRecv |= RTSERIALPORT_EVT_F_BREAK_DETECTED;
-            if (dwEventMask & (EV_CTS | EV_DSR | EV_RING | EV_RLSD))
-                *pfEvtsRecv |= RTSERIALPORT_EVT_F_STATUS_LINE_CHANGED;
+            pThis->fEvtQueryPending = false;
+            rc = rtSerialPortEvtFlagsProcess(pThis, pThis->dwEventMask, pfEvtsRecv);
         }
     }