Changeset 75979 in vbox

Timestamp:

Dec 5, 2018 3:50:48 PM (6 years ago)

Author:

vboxsync

Message:

VMMDevHGCM: It is not possible to restore cancelled requests (guest memory no longer valid) and it makes no sense to even save them. Fixes issues like 'VBoxControl guestproperty wait' failing with VERR_HGCM_INVALID_CLIENT_ID on restore.

File:

• trunk/src/VBox/Devices/VMMDev/VMMDevHGCM.cpp (modified) (6 diffs)

trunk/src/VBox/Devices/VMMDev/VMMDevHGCM.cpp

@@ -1812 +1812 @@
             AssertRCReturn(rc, rc);
 
-            vmmdevHGCMAddCommand(pThis, pCmd);
+            /*
+             * Do not restore cancelled calls.  Why do we save them to start with?
+             *
+             * The guest memory no longer contains a valid request!  So, it is not
+             * possible to restore it.  The memory is often reused for a new request
+             * by now and we will end up trying to complete that more than once if
+             * we restore a cancelled call.  In some cases VERR_HGCM_INVALID_CLIENT_ID
+             * is returned, though it might just be silent memory corruption.
+             */
+            /* See current version above. */
+            if (!fCancelled)
+                vmmdevHGCMAddCommand(pThis, pCmd);
+            else
+            {
+                Log(("vmmdevHGCMLoadState: Skipping cancelled request: enmCmdType=%d GCPhys=%#RX32 LB %#x\n",
+                     enmCmdType, GCPhys, cbRequest));
+                vmmdevHGCMCmdFree(pThis, pCmd);
+            }
         }
 
@@ -1888 +1905 @@
             AssertRCReturn(rc, rc);
 
-            vmmdevHGCMAddCommand(pThis, pCmd);
+            /* See current version above. */
+            if (!fCancelled)
+                vmmdevHGCMAddCommand(pThis, pCmd);
+            else
+            {
+                Log(("vmmdevHGCMLoadState: Skipping cancelled request: enmCmdType=%d GCPhys=%#RX32 LB %#x\n",
+                     enmCmdType, GCPhys, cbRequest));
+                vmmdevHGCMCmdFree(pThis, pCmd);
+            }
         }
 
@@ -1948 +1973 @@
     AssertReturn(pCmd, VERR_NO_MEMORY);
 
-    if (u32SSMVersion >= 9)
-        pCmd->fCancelled = pLoadedCmd->fCancelled;
-    else
-        pCmd->fCancelled = false;
+    Assert(pLoadedCmd->fCancelled == false);
+    pCmd->fCancelled = false;
     pCmd->fRestored      = true;
     pCmd->enmRequestType = enmRequestType;
@@ -1991 +2014 @@
     AssertReturn(pCmd, VERR_NO_MEMORY);
 
-    if (u32SSMVersion >= 9)
-        pCmd->fCancelled = pLoadedCmd->fCancelled;
-    else
-        pCmd->fCancelled = false;
+    Assert(pLoadedCmd->fCancelled == false);
+    pCmd->fCancelled = false;
     pCmd->fRestored      = true;
     pCmd->enmRequestType = enmRequestType;
@@ -2028 +2049 @@
     {
         ASSERT_GUEST_RETURN(pLoadedCmd->enmCmdType == VBOXHGCMCMDTYPE_CALL, VERR_MISMATCH);
+        Assert(pLoadedCmd->fCancelled == false);
     }
 
@@ -2037 +2059 @@
 
     /* pLoadedCmd is fake, it does not contain actual call parameters. Only pagelists for LinAddr. */
-    if (u32SSMVersion >= 9)
-        pCmd->fCancelled = pLoadedCmd->fCancelled;
-    else
-        pCmd->fCancelled = false;
+    pCmd->fCancelled = false;
     pCmd->fRestored      = true;
     pCmd->enmRequestType = enmRequestType;