Changeset 97610 in vbox for trunk/src/VBox

Timestamp:

Nov 18, 2022 5:29:35 PM (2 years ago)

Author:

vboxsync

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

154654

Message:

Guest Control/Main: Also catch (and handle) OOM situations in GuestFile notification callbacks better.

File:

• trunk/src/VBox/Main/src-client/GuestFileImpl.cpp (modified) (7 diffs)

trunk/src/VBox/Main/src-client/GuestFileImpl.cpp

@@ -534 +534 @@
         case GUEST_FILE_NOTIFYTYPE_READ:
         {
-            if (pSvcCbData->mParms == 4)
+            ASSERT_GUEST_MSG_STMT_BREAK(pSvcCbData->mParms == 4, ("mParms=%u\n", pSvcCbData->mParms),
+                                        vrc = VERR_WRONG_PARAMETER_COUNT);
+            ASSERT_GUEST_MSG_STMT_BREAK(pSvcCbData->mpaParms[idx].type == VBOX_HGCM_SVC_PARM_PTR,
+                                        ("type=%u\n", pSvcCbData->mpaParms[idx].type),
+                                        vrc = VERR_WRONG_PARAMETER_TYPE);
+
+            vrc = HGCMSvcGetPv(&pSvcCbData->mpaParms[idx++], &dataCb.u.read.pvData, &dataCb.u.read.cbData);
+            if (RT_FAILURE(vrc))
+                break;
+
+            const uint32_t cbRead = dataCb.u.read.cbData;
+            Log3ThisFunc(("cbRead=%RU32\n", cbRead));
+
+            AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+            mData.mOffCurrent += cbRead; /* Bogus for readAt, which is why we've got GUEST_FILE_NOTIFYTYPE_READ_OFFSET. */
+            alock.release();
+
+            try
             {
-                vrc = HGCMSvcGetPv(&pSvcCbData->mpaParms[idx++], &dataCb.u.read.pvData,
-                                  &dataCb.u.read.cbData);
-                if (RT_FAILURE(vrc))
-                    break;
-
-                const uint32_t cbRead = dataCb.u.read.cbData;
-
-                Log3ThisFunc(("cbRead=%RU32\n", cbRead));
-
-                AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-                mData.mOffCurrent += cbRead; /* Bogus for readAt, which is why we've got GUEST_FILE_NOTIFYTYPE_READ_OFFSET. */
-
-                alock.release();
-
                 com::SafeArray<BYTE> data((size_t)cbRead);
+                AssertBreakStmt(data.size() == cbRead, vrc = VERR_NO_MEMORY);
                 data.initFrom((BYTE *)dataCb.u.read.pvData, cbRead);
-
                 ::FireGuestFileReadEvent(mEventSource, mSession, this, mData.mOffCurrent, cbRead, ComSafeArrayAsInParam(data));
+            }
+            catch (std::bad_alloc &)
+            {
+                vrc = VERR_NO_MEMORY;
             }
             break;
@@ -567 +573 @@
                                         vrc = VERR_WRONG_PARAMETER_TYPE);
             ASSERT_GUEST_MSG_STMT_BREAK(pSvcCbData->mpaParms[idx + 1].type == VBOX_HGCM_SVC_PARM_64BIT,
-                                        ("type=%u\n", pSvcCbData->mpaParms[idx].type),
+                                        ("type=%u\n", pSvcCbData->mpaParms[idx + 1].type),
                                         vrc = VERR_WRONG_PARAMETER_TYPE);
             BYTE const * const pbData = (BYTE const *)pSvcCbData->mpaParms[idx].u.pointer.addr;
@@ -583 +589 @@
             {
                 com::SafeArray<BYTE> data((size_t)cbRead);
+                AssertBreakStmt(data.size() == cbRead, vrc = VERR_NO_MEMORY);
                 data.initFrom(pbData, cbRead);
                 ::FireGuestFileReadEvent(mEventSource, mSession, this, offNew, cbRead, ComSafeArrayAsInParam(data));
@@ -596 +603 @@
         case GUEST_FILE_NOTIFYTYPE_WRITE:
         {
-            if (pSvcCbData->mParms == 4)
-            {
-                vrc = HGCMSvcGetU32(&pSvcCbData->mpaParms[idx++], &dataCb.u.write.cbWritten);
-                if (RT_FAILURE(vrc))
-                    break;
-
-                const uint32_t cbWritten = dataCb.u.write.cbWritten;
-
-                Log3ThisFunc(("cbWritten=%RU32\n", cbWritten));
-
-                AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-                mData.mOffCurrent += cbWritten; /* Bogus for writeAt and append mode, thus GUEST_FILE_NOTIFYTYPE_WRITE_OFFSET. */
-
-                alock.release();
-
-                ::FireGuestFileWriteEvent(mEventSource, mSession, this, mData.mOffCurrent, cbWritten);
-            }
+            ASSERT_GUEST_MSG_STMT_BREAK(pSvcCbData->mParms == 4, ("mParms=%u\n", pSvcCbData->mParms),
+                                        vrc = VERR_WRONG_PARAMETER_COUNT);
+            ASSERT_GUEST_MSG_STMT_BREAK(pSvcCbData->mpaParms[idx].type == VBOX_HGCM_SVC_PARM_32BIT,
+                                        ("type=%u\n", pSvcCbData->mpaParms[idx].type),
+                                        vrc = VERR_WRONG_PARAMETER_TYPE);
+
+            uint32_t const cbWritten = pSvcCbData->mpaParms[idx].u.uint32;
+
+            Log3ThisFunc(("cbWritten=%RU32\n", cbWritten));
+
+            AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+            mData.mOffCurrent += cbWritten; /* Bogus for writeAt and append mode, thus GUEST_FILE_NOTIFYTYPE_WRITE_OFFSET. */
+            alock.release();
+
+            ::FireGuestFileWriteEvent(mEventSource, mSession, this, mData.mOffCurrent, cbWritten);
             break;
         }
@@ -644 +648 @@
         case GUEST_FILE_NOTIFYTYPE_SEEK:
         {
-            if (pSvcCbData->mParms == 4)
-            {
-                vrc = HGCMSvcGetU64(&pSvcCbData->mpaParms[idx++], &dataCb.u.seek.uOffActual);
-                if (RT_FAILURE(vrc))
-                    break;
-
-                Log3ThisFunc(("uOffActual=%RU64\n", dataCb.u.seek.uOffActual));
-
-                AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-                mData.mOffCurrent = dataCb.u.seek.uOffActual;
-
-                alock.release();
-
-                ::FireGuestFileOffsetChangedEvent(mEventSource, mSession, this, dataCb.u.seek.uOffActual, 0 /* Processed */);
-            }
+            ASSERT_GUEST_MSG_STMT_BREAK(pSvcCbData->mParms == 4, ("mParms=%u\n", pSvcCbData->mParms),
+                                        vrc = VERR_WRONG_PARAMETER_COUNT);
+
+            vrc = HGCMSvcGetU64(&pSvcCbData->mpaParms[idx++], &dataCb.u.seek.uOffActual);
+            if (RT_FAILURE(vrc))
+                break;
+
+            Log3ThisFunc(("uOffActual=%RU64\n", dataCb.u.seek.uOffActual));
+
+            AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+            mData.mOffCurrent = dataCb.u.seek.uOffActual;
+            alock.release();
+
+            ::FireGuestFileOffsetChangedEvent(mEventSource, mSession, this, dataCb.u.seek.uOffActual, 0 /* Processed */);
             break;
         }
@@ -685 +687 @@
     }
 
-    if (RT_SUCCESS(vrc))
-    {
-        try
+    try
+    {
+        if (RT_SUCCESS(vrc))
         {
             GuestWaitEventPayload payload(dataCb.uType, &dataCb, sizeof(dataCb));
@@ -694 +696 @@
             signalWaitEventInternal(pCbCtx, vrcGuest, &payload);
         }
-        catch (int vrcEx) /* Thrown by GuestWaitEventPayload constructor. */
-        {
-            vrc = vrcEx;
-        }
+        else /* OOM situation, wrong HGCM parameters or smth. not expected. */
+        {
+            /* Ignore rc, as the event to signal might not be there (anymore). */
+            signalWaitEventInternalEx(pCbCtx, vrc, 0 /* guestRc */, NULL /* pPayload */);
+        }
+    }
+    catch (int vrcEx) /* Thrown by GuestWaitEventPayload constructor. */
+    {
+        /* Also try to signal the waiter, to let it know of the OOM situation.
+         * Ignore rc, as the event to signal might not be there (anymore). */
+        signalWaitEventInternalEx(pCbCtx, vrcEx, 0 /* guestRc */, NULL /* pPayload */);
+        vrc = vrcEx;
     }