Imported from /home/lorry/working-area/delta_VirtualBox/VirtualBox-4.3.10.tar.bz2.HEADVirtualBox-4.3.10master

1 files changed, 323 insertions, 142 deletions

diff --git a/src/VBox/Devices/VMMDev/VMMDevHGCM.cpp b/src/VBox/Devices/VMMDev/VMMDevHGCM.cpp
index 4a531a89..ca942b1e 100644
--- a/src/VBox/Devices/VMMDev/VMMDevHGCM.cpp
+++ b/src/VBox/Devices/VMMDev/VMMDevHGCM.cpp
@@ -4,7 +4,7 @@
  */
 
 /*
- * Copyright (C) 2006-2007 Oracle Corporation
+ * Copyright (C) 2006-2013 Oracle Corporation
  *
  * This file is part of VirtualBox Open Source Edition (OSE), as
  * available from http://www.virtualbox.org. This file is free software;
@@ -16,6 +16,9 @@
  */
 
 
+/*******************************************************************************
+*   Header Files                                                               *
+*******************************************************************************/
 #define LOG_GROUP LOG_GROUP_DEV_VMM
 #include <iprt/alloc.h>
 #include <iprt/asm.h>
@@ -39,7 +42,11 @@
 # define VBOXDD_HGCMCALL_COMPLETED_DONE(a,b,c,d)    do { } while (0)
 #endif
 
-typedef enum _VBOXHGCMCMDTYPE
+
+/*******************************************************************************
+*   Structures and Typedefs                                                    *
+*******************************************************************************/
+typedef enum VBOXHGCMCMDTYPE
 {
     VBOXHGCMCMDTYPE_LOADSTATE = 0,
     VBOXHGCMCMDTYPE_CONNECT,
@@ -48,21 +55,23 @@ typedef enum _VBOXHGCMCMDTYPE
     VBOXHGCMCMDTYPE_SizeHack = 0x7fffffff
 } VBOXHGCMCMDTYPE;
 
-/* Information about a linear ptr parameter. */
-typedef struct _VBOXHGCMLINPTR
+/**
+ * Information about a linear ptr parameter.
+ */
+typedef struct VBOXHGCMLINPTR
 {
-    /* Index of the parameter. */
+    /** Index of the parameter. */
     uint32_t iParm;
 
-    /* Offset in the first physical page of the region. */
+    /** Offset in the first physical page of the region. */
     uint32_t offFirstPage;
 
-    /* How many pages. */
+    /** How many pages. */
     uint32_t cPages;
 
-    /* Pointer to array of the GC physical addresses for these pages.
-     * It is assumed that the physical address of the locked resident
-     * guest page does not change.
+    /** Pointer to array of the GC physical addresses for these pages.
+     * It is assumed that the physical address of the locked resident guest page
+     * does not change.
      */
     RTGCPHYS *paPages;
 
@@ -70,78 +79,83 @@ typedef struct _VBOXHGCMLINPTR
 
 struct VBOXHGCMCMD
 {
-    /* Active commands, list is protected by critsectHGCMCmdList. */
+    /** Active commands, list is protected by critsectHGCMCmdList. */
     struct VBOXHGCMCMD *pNext;
     struct VBOXHGCMCMD *pPrev;
 
-    /* The type of the command. */
+    /** The type of the command. */
     VBOXHGCMCMDTYPE enmCmdType;
 
-    /* Whether the command was cancelled by the guest. */
+    /** Whether the command was cancelled by the guest. */
     bool fCancelled;
 
-    /* Whether the command is in the active commands list. */
+    /** Whether the command is in the active commands list. */
     bool fInList;
 
-    /* Whether the command was saved. */
+    /** Whether the command was saved. */
     bool fSaved;
 
-    /* GC physical address of the guest request. */
+    /** GC physical address of the guest request. */
     RTGCPHYS        GCPhys;
 
-    /* Request packet size */
+    /** Request packet size */
     uint32_t        cbSize;
 
-    /* Pointer to converted host parameters in case of a Call request.
+    /** Pointer to converted host parameters in case of a Call request.
      * Parameters follow this structure in the same memory block.
      */
     VBOXHGCMSVCPARM *paHostParms;
 
-    /* Linear pointer parameters information. */
+    /* Number of elements in paHostParms */
+    uint32_t cHostParms;
+
+    /** Linear pointer parameters information. */
     int cLinPtrs;
 
-    /* How many pages for all linptrs of this command.
+    /** How many pages for all linptrs of this command.
      * Only valid if cLinPtrs > 0. This field simplifies loading of saved state.
      */
     int cLinPtrPages;
 
-    /* Pointer to descriptions of linear pointers.  */
+    /** Pointer to descriptions of linear pointers.  */
     VBOXHGCMLINPTR *paLinPtrs;
 };
 
-static int vmmdevHGCMCmdListLock (VMMDevState *pVMMDevState)
+
+
+static int vmmdevHGCMCmdListLock (PVMMDEV pThis)
 {
-    int rc = RTCritSectEnter (&pVMMDevState->critsectHGCMCmdList);
+    int rc = RTCritSectEnter (&pThis->critsectHGCMCmdList);
     AssertRC (rc);
     return rc;
 }
 
-static void vmmdevHGCMCmdListUnlock (VMMDevState *pVMMDevState)
+static void vmmdevHGCMCmdListUnlock (PVMMDEV pThis)
 {
-    int rc = RTCritSectLeave (&pVMMDevState->critsectHGCMCmdList);
+    int rc = RTCritSectLeave (&pThis->critsectHGCMCmdList);
     AssertRC (rc);
 }
 
-static int vmmdevHGCMAddCommand (VMMDevState *pVMMDevState, PVBOXHGCMCMD pCmd, RTGCPHYS GCPhys, uint32_t cbSize, VBOXHGCMCMDTYPE enmCmdType)
+static int vmmdevHGCMAddCommand (PVMMDEV pThis, PVBOXHGCMCMD pCmd, RTGCPHYS GCPhys, uint32_t cbSize, VBOXHGCMCMDTYPE enmCmdType)
 {
-    /* PPDMDEVINS pDevIns = pVMMDevState->pDevIns; */
+    /* PPDMDEVINS pDevIns = pThis->pDevIns; */
 
-    int rc = vmmdevHGCMCmdListLock (pVMMDevState);
+    int rc = vmmdevHGCMCmdListLock (pThis);
 
     if (RT_SUCCESS (rc))
     {
         LogFlowFunc(("%p type %d\n", pCmd, enmCmdType));
 
         /* Insert at the head of the list. The vmmdevHGCMLoadStateDone depends on this. */
-        pCmd->pNext = pVMMDevState->pHGCMCmdList;
+        pCmd->pNext = pThis->pHGCMCmdList;
         pCmd->pPrev = NULL;
 
-        if (pVMMDevState->pHGCMCmdList)
+        if (pThis->pHGCMCmdList)
         {
-            pVMMDevState->pHGCMCmdList->pPrev = pCmd;
+            pThis->pHGCMCmdList->pPrev = pCmd;
         }
 
-        pVMMDevState->pHGCMCmdList = pCmd;
+        pThis->pHGCMCmdList = pCmd;
 
         pCmd->fInList = true;
 
@@ -158,24 +172,24 @@ static int vmmdevHGCMAddCommand (VMMDevState *pVMMDevState, PVBOXHGCMCMD pCmd, R
             || enmCmdType == VBOXHGCMCMDTYPE_DISCONNECT
             || enmCmdType == VBOXHGCMCMDTYPE_CALL)
         {
-            Log(("vmmdevHGCMAddCommand: u32HGCMEnabled = %d\n", pVMMDevState->u32HGCMEnabled));
-            if (ASMAtomicCmpXchgU32(&pVMMDevState->u32HGCMEnabled, 1, 0))
+            Log(("vmmdevHGCMAddCommand: u32HGCMEnabled = %d\n", pThis->u32HGCMEnabled));
+            if (ASMAtomicCmpXchgU32(&pThis->u32HGCMEnabled, 1, 0))
             {
-                 VMMDevCtlSetGuestFilterMask (pVMMDevState, VMMDEV_EVENT_HGCM, 0);
+                 VMMDevCtlSetGuestFilterMask (pThis, VMMDEV_EVENT_HGCM, 0);
             }
         }
 
-        vmmdevHGCMCmdListUnlock (pVMMDevState);
+        vmmdevHGCMCmdListUnlock (pThis);
     }
 
     return rc;
 }
 
-static int vmmdevHGCMRemoveCommand (VMMDevState *pVMMDevState, PVBOXHGCMCMD pCmd)
+static int vmmdevHGCMRemoveCommand (PVMMDEV pThis, PVBOXHGCMCMD pCmd)
 {
-    /* PPDMDEVINS pDevIns = pVMMDevState->pDevIns; */
+    /* PPDMDEVINS pDevIns = pThis->pDevIns; */
 
-    int rc = vmmdevHGCMCmdListLock (pVMMDevState);
+    int rc = vmmdevHGCMCmdListLock (pThis);
 
     if (RT_SUCCESS (rc))
     {
@@ -184,7 +198,7 @@ static int vmmdevHGCMRemoveCommand (VMMDevState *pVMMDevState, PVBOXHGCMCMD pCmd
         if (!pCmd->fInList)
         {
             LogFlowFunc(("%p not in the list\n", pCmd));
-            vmmdevHGCMCmdListUnlock (pVMMDevState);
+            vmmdevHGCMCmdListUnlock (pThis);
             return VINF_SUCCESS;
         }
 
@@ -203,14 +217,14 @@ static int vmmdevHGCMRemoveCommand (VMMDevState *pVMMDevState, PVBOXHGCMCMD pCmd
         }
         else
         {
-            pVMMDevState->pHGCMCmdList = pCmd->pNext;
+            pThis->pHGCMCmdList = pCmd->pNext;
         }
 
         pCmd->pNext = NULL;
         pCmd->pPrev = NULL;
         pCmd->fInList = false;
 
-        vmmdevHGCMCmdListUnlock (pVMMDevState);
+        vmmdevHGCMCmdListUnlock (pThis);
     }
 
     return rc;
@@ -228,7 +242,7 @@ static int vmmdevHGCMRemoveCommand (VMMDevState *pVMMDevState, PVBOXHGCMCMD pCmd
  * @param   GCPhys          The physical address of the command we're looking
  *                          for.
  */
-DECLINLINE(PVBOXHGCMCMD) vmmdevHGCMFindCommandLocked (VMMDevState *pThis, RTGCPHYS GCPhys)
+DECLINLINE(PVBOXHGCMCMD) vmmdevHGCMFindCommandLocked (PVMMDEV pThis, RTGCPHYS GCPhys)
 {
     for (PVBOXHGCMCMD pCmd = pThis->pHGCMCmdList;
          pCmd;
@@ -250,8 +264,6 @@ static int vmmdevHGCMSaveLinPtr (PPDMDEVINS pDevIns,
 {
     int rc = VINF_SUCCESS;
 
-    AssertRelease (u32Size > 0);
-
     VBOXHGCMLINPTR *pLinPtr = &paLinPtrs[iLinPtr];
 
     /* Take the offset into the current page also into account! */
@@ -294,8 +306,6 @@ static int vmmdevHGCMSaveLinPtr (PPDMDEVINS pDevIns,
         GCPtr += PAGE_SIZE;
     }
 
-    AssertRelease (iPage == cPages);
-
     return rc;
 }
 
@@ -310,7 +320,7 @@ static int vmmdevHGCMWriteLinPtr (PPDMDEVINS pDevIns,
 
     VBOXHGCMLINPTR *pLinPtr = &paLinPtrs[iLinPtr];
 
-    AssertRelease (u32Size > 0 && iParm == (uint32_t)pLinPtr->iParm);
+    AssertLogRelReturn(u32Size > 0 && iParm == (uint32_t)pLinPtr->iParm, VERR_INVALID_PARAMETER);
 
     RTGCPHYS GCPhysDst = pLinPtr->paPages[0] + pLinPtr->offFirstPage;
     uint8_t *pu8Src    = (uint8_t *)pvHost;
@@ -332,12 +342,17 @@ static int vmmdevHGCMWriteLinPtr (PPDMDEVINS pDevIns,
 
         if (cbWrite >= u32Size)
         {
-            PDMDevHlpPhysWrite(pDevIns, GCPhysDst, pu8Src, u32Size);
+            rc = PDMDevHlpPhysWrite(pDevIns, GCPhysDst, pu8Src, u32Size);
+            if (RT_FAILURE(rc))
+                break;
+
             u32Size = 0;
             break;
         }
 
-        PDMDevHlpPhysWrite(pDevIns, GCPhysDst, pu8Src, cbWrite);
+        rc = PDMDevHlpPhysWrite(pDevIns, GCPhysDst, pu8Src, cbWrite);
+        if (RT_FAILURE(rc))
+            break;
 
         /* next */
         u32Size    -= cbWrite;
@@ -346,8 +361,10 @@ static int vmmdevHGCMWriteLinPtr (PPDMDEVINS pDevIns,
         GCPhysDst   = pLinPtr->paPages[iPage];
     }
 
-    AssertRelease (iPage == pLinPtr->cPages);
-    Assert(u32Size == 0);
+    if (RT_SUCCESS(rc))
+    {
+        AssertLogRelReturn(iPage == pLinPtr->cPages, VERR_INVALID_PARAMETER);
+    }
 
     return rc;
 }
@@ -473,7 +490,7 @@ static void vmmdevRestoreSavedCommand(VBOXHGCMCMD *pCmd, VBOXHGCMCMD *pSavedCmd)
     pSavedCmd->paLinPtrs = NULL;
 }
 
-int vmmdevHGCMConnect (VMMDevState *pVMMDevState, VMMDevHGCMConnect *pHGCMConnect, RTGCPHYS GCPhys)
+int vmmdevHGCMConnect (PVMMDEV pThis, VMMDevHGCMConnect *pHGCMConnect, RTGCPHYS GCPhys)
 {
     int rc = VINF_SUCCESS;
 
@@ -485,7 +502,7 @@ int vmmdevHGCMConnect (VMMDevState *pVMMDevState, VMMDevHGCMConnect *pHGCMConnec
     {
         VMMDevHGCMConnect *pHGCMConnectCopy = (VMMDevHGCMConnect *)(pCmd+1);
 
-        vmmdevHGCMAddCommand (pVMMDevState, pCmd, GCPhys, pHGCMConnect->header.header.size, VBOXHGCMCMDTYPE_CONNECT);
+        vmmdevHGCMAddCommand (pThis, pCmd, GCPhys, pHGCMConnect->header.header.size, VBOXHGCMCMDTYPE_CONNECT);
 
         memcpy(pHGCMConnectCopy, pHGCMConnect, pHGCMConnect->header.header.size);
 
@@ -497,7 +514,10 @@ int vmmdevHGCMConnect (VMMDevState *pVMMDevState, VMMDevHGCMConnect *pHGCMConnec
         Assert(pHGCMConnectCopy->loc.type == VMMDevHGCMLoc_LocalHost_Existing);
         pHGCMConnectCopy->loc.type = VMMDevHGCMLoc_LocalHost_Existing;
 
-        rc = pVMMDevState->pHGCMDrv->pfnConnect (pVMMDevState->pHGCMDrv, pCmd, &pHGCMConnectCopy->loc, &pHGCMConnectCopy->u32ClientID);
+        rc = pThis->pHGCMDrv->pfnConnect (pThis->pHGCMDrv, pCmd, &pHGCMConnectCopy->loc, &pHGCMConnectCopy->u32ClientID);
+
+        if (RT_FAILURE(rc))
+            vmmdevHGCMRemoveCommand(pThis, pCmd);
     }
     else
     {
@@ -507,7 +527,7 @@ int vmmdevHGCMConnect (VMMDevState *pVMMDevState, VMMDevHGCMConnect *pHGCMConnec
     return rc;
 }
 
-static int vmmdevHGCMConnectSaved (VMMDevState *pVMMDevState, VMMDevHGCMConnect *pHGCMConnect, RTGCPHYS GCPhys, bool *pfHGCMCalled, VBOXHGCMCMD *pSavedCmd, VBOXHGCMCMD **ppCmd)
+static int vmmdevHGCMConnectSaved (PVMMDEV pThis, VMMDevHGCMConnect *pHGCMConnect, RTGCPHYS GCPhys, bool *pfHGCMCalled, VBOXHGCMCMD *pSavedCmd, VBOXHGCMCMD **ppCmd)
 {
     int rc = VINF_SUCCESS;
 
@@ -523,7 +543,7 @@ static int vmmdevHGCMConnectSaved (VMMDevState *pVMMDevState, VMMDevHGCMConnect
 
         VMMDevHGCMConnect *pHGCMConnectCopy = (VMMDevHGCMConnect *)(pCmd+1);
 
-        vmmdevHGCMAddCommand (pVMMDevState, pCmd, GCPhys, pHGCMConnect->header.header.size, VBOXHGCMCMDTYPE_CONNECT);
+        vmmdevHGCMAddCommand (pThis, pCmd, GCPhys, pHGCMConnect->header.header.size, VBOXHGCMCMDTYPE_CONNECT);
 
         memcpy(pHGCMConnectCopy, pHGCMConnect, pHGCMConnect->header.header.size);
 
@@ -531,12 +551,14 @@ static int vmmdevHGCMConnectSaved (VMMDevState *pVMMDevState, VMMDevHGCMConnect
         Assert(pHGCMConnectCopy->loc.type == VMMDevHGCMLoc_LocalHost_Existing);
         pHGCMConnectCopy->loc.type = VMMDevHGCMLoc_LocalHost_Existing;
 
-        rc = pVMMDevState->pHGCMDrv->pfnConnect (pVMMDevState->pHGCMDrv, pCmd, &pHGCMConnectCopy->loc, &pHGCMConnectCopy->u32ClientID);
+        rc = pThis->pHGCMDrv->pfnConnect (pThis->pHGCMDrv, pCmd, &pHGCMConnectCopy->loc, &pHGCMConnectCopy->u32ClientID);
 
         if (RT_SUCCESS (rc))
         {
             *pfHGCMCalled = true;
         }
+        /* else
+         *  ... the caller will also execute vmmdevHGCMRemoveCommand() for us */
     }
     else
     {
@@ -546,7 +568,7 @@ static int vmmdevHGCMConnectSaved (VMMDevState *pVMMDevState, VMMDevHGCMConnect
     return rc;
 }
 
-int vmmdevHGCMDisconnect (VMMDevState *pVMMDevState, VMMDevHGCMDisconnect *pHGCMDisconnect, RTGCPHYS GCPhys)
+int vmmdevHGCMDisconnect (PVMMDEV pThis, VMMDevHGCMDisconnect *pHGCMDisconnect, RTGCPHYS GCPhys)
 {
     int rc = VINF_SUCCESS;
 
@@ -556,13 +578,16 @@ int vmmdevHGCMDisconnect (VMMDevState *pVMMDevState, VMMDevHGCMDisconnect *pHGCM
 
     if (pCmd)
     {
-        vmmdevHGCMAddCommand (pVMMDevState, pCmd, GCPhys, pHGCMDisconnect->header.header.size, VBOXHGCMCMDTYPE_DISCONNECT);
+        vmmdevHGCMAddCommand (pThis, pCmd, GCPhys, pHGCMDisconnect->header.header.size, VBOXHGCMCMDTYPE_DISCONNECT);
 
         pCmd->paHostParms = NULL;
         pCmd->cLinPtrs = 0;
         pCmd->paLinPtrs = NULL;
 
-        rc = pVMMDevState->pHGCMDrv->pfnDisconnect (pVMMDevState->pHGCMDrv, pCmd, pHGCMDisconnect->u32ClientID);
+        rc = pThis->pHGCMDrv->pfnDisconnect (pThis->pHGCMDrv, pCmd, pHGCMDisconnect->u32ClientID);
+
+        if (RT_FAILURE(rc))
+            vmmdevHGCMRemoveCommand(pThis, pCmd);
     }
     else
     {
@@ -572,7 +597,7 @@ int vmmdevHGCMDisconnect (VMMDevState *pVMMDevState, VMMDevHGCMDisconnect *pHGCM
     return rc;
 }
 
-static int vmmdevHGCMDisconnectSaved (VMMDevState *pVMMDevState, VMMDevHGCMDisconnect *pHGCMDisconnect, RTGCPHYS GCPhys, bool *pfHGCMCalled, VBOXHGCMCMD *pSavedCmd, VBOXHGCMCMD **ppCmd)
+static int vmmdevHGCMDisconnectSaved (PVMMDEV pThis, VMMDevHGCMDisconnect *pHGCMDisconnect, RTGCPHYS GCPhys, bool *pfHGCMCalled, VBOXHGCMCMD *pSavedCmd, VBOXHGCMCMD **ppCmd)
 {
     int rc = VINF_SUCCESS;
 
@@ -585,18 +610,20 @@ static int vmmdevHGCMDisconnectSaved (VMMDevState *pVMMDevState, VMMDevHGCMDisco
         vmmdevRestoreSavedCommand(pCmd, pSavedCmd);
         *ppCmd = pCmd;
 
-        vmmdevHGCMAddCommand (pVMMDevState, pCmd, GCPhys, pHGCMDisconnect->header.header.size, VBOXHGCMCMDTYPE_DISCONNECT);
+        vmmdevHGCMAddCommand (pThis, pCmd, GCPhys, pHGCMDisconnect->header.header.size, VBOXHGCMCMDTYPE_DISCONNECT);
 
         pCmd->paHostParms = NULL;
         pCmd->cLinPtrs = 0;
         pCmd->paLinPtrs = NULL;
 
-        rc = pVMMDevState->pHGCMDrv->pfnDisconnect (pVMMDevState->pHGCMDrv, pCmd, pHGCMDisconnect->u32ClientID);
+        rc = pThis->pHGCMDrv->pfnDisconnect (pThis->pHGCMDrv, pCmd, pHGCMDisconnect->u32ClientID);
 
         if (RT_SUCCESS (rc))
         {
             *pfHGCMCalled = true;
         }
+        /* else
+         *  ... the caller will also execute vmmdevHGCMRemoveCommand() for us */
     }
     else
     {
@@ -606,7 +633,7 @@ static int vmmdevHGCMDisconnectSaved (VMMDevState *pVMMDevState, VMMDevHGCMDisco
     return rc;
 }
 
-int vmmdevHGCMCall (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCMCall, uint32_t cbHGCMCall, RTGCPHYS GCPhys, bool f64Bits)
+int vmmdevHGCMCall (PVMMDEV pThis, VMMDevHGCMCall *pHGCMCall, uint32_t cbHGCMCall, RTGCPHYS GCPhys, bool f64Bits)
 {
     int rc = VINF_SUCCESS;
 
@@ -623,6 +650,20 @@ int vmmdevHGCMCall (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCMCall, uint32
     Log(("vmmdevHGCMCall: cParms = %d\n", cParms));
 
     /*
+     * Sane upper limit.
+     */
+    if (cParms > VMMDEV_MAX_HGCM_PARMS)
+    {
+        static int s_cRelWarn;
+        if (s_cRelWarn < 50)
+        {
+            s_cRelWarn++;
+            LogRel(("VMMDev: request packet with too many parameters (%d). Refusing operation.\n", cParms));
+        }
+        return VERR_INVALID_PARAMETER;
+    }
+
+    /*
      * Compute size of required memory buffer.
      */
 
@@ -654,6 +695,12 @@ int vmmdevHGCMCall (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCMCall, uint32
                     if (pGuestParm->u.Pointer.size > 0)
                     {
                         /* Only pointers with some actual data are counted. */
+                        if (pGuestParm->u.Pointer.size > VMMDEV_MAX_HGCM_DATA_SIZE - cbCmdSize)
+                        {
+                            rc = VERR_INVALID_PARAMETER;
+                            break;
+                        }
+
                         cbCmdSize += pGuestParm->u.Pointer.size;
 
                         cLinPtrs++;
@@ -667,6 +714,12 @@ int vmmdevHGCMCall (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCMCall, uint32
 
                 case VMMDevHGCMParmType_PageList:
                 {
+                    if (pGuestParm->u.PageList.size > VMMDEV_MAX_HGCM_DATA_SIZE - cbCmdSize)
+                    {
+                        rc = VERR_INVALID_PARAMETER;
+                        break;
+                    }
+
                     cbCmdSize += pGuestParm->u.PageList.size;
                     Log(("vmmdevHGCMCall: pagelist size = %d\n", pGuestParm->u.PageList.size));
                 } break;
@@ -706,6 +759,12 @@ int vmmdevHGCMCall (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCMCall, uint32
                     if (pGuestParm->u.Pointer.size > 0)
                     {
                         /* Only pointers with some actual data are counted. */
+                        if (pGuestParm->u.Pointer.size > VMMDEV_MAX_HGCM_DATA_SIZE - cbCmdSize)
+                        {
+                            rc = VERR_INVALID_PARAMETER;
+                            break;
+                        }
+
                         cbCmdSize += pGuestParm->u.Pointer.size;
 
                         cLinPtrs++;
@@ -719,6 +778,12 @@ int vmmdevHGCMCall (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCMCall, uint32
 
                 case VMMDevHGCMParmType_PageList:
                 {
+                    if (pGuestParm->u.PageList.size > VMMDEV_MAX_HGCM_DATA_SIZE - cbCmdSize)
+                    {
+                        rc = VERR_INVALID_PARAMETER;
+                        break;
+                    }
+
                     cbCmdSize += pGuestParm->u.PageList.size;
                     Log(("vmmdevHGCMCall: pagelist size = %d\n", pGuestParm->u.PageList.size));
                 } break;
@@ -787,6 +852,7 @@ int vmmdevHGCMCall (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCMCall, uint32
         uint8_t *pcBuf = (uint8_t *)pHostParm + cParms * sizeof (VBOXHGCMSVCPARM);
 
         pCmd->paHostParms = pHostParm;
+        pCmd->cHostParms  = cParms;
 
         uint32_t iLinPtr = 0;
         RTGCPHYS *pPages  = (RTGCPHYS *)((uint8_t *)pCmd->paLinPtrs + sizeof (VBOXHGCMLINPTR) *cLinPtrs);
@@ -849,7 +915,7 @@ int vmmdevHGCMCall (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCMCall, uint32
                          {
                              /* Don't overdo it */
                              if (pGuestParm->type != VMMDevHGCMParmType_LinAddr_Out)
-                                 rc = PDMDevHlpPhysReadGCVirt(pVMMDevState->pDevIns, pcBuf, linearAddr, size);
+                                 rc = PDMDevHlpPhysReadGCVirt(pThis->pDevIns, pcBuf, linearAddr, size);
                              else
                                  rc = VINF_SUCCESS;
 
@@ -861,7 +927,7 @@ int vmmdevHGCMCall (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCMCall, uint32
                                  /* Remember the guest physical pages that belong to the virtual address region.
                                   * Do it for all linear pointers because load state will require In pointer info too.
                                   */
-                                 rc = vmmdevHGCMSaveLinPtr (pVMMDevState->pDevIns, i, linearAddr, size, iLinPtr, pCmd->paLinPtrs, &pPages);
+                                 rc = vmmdevHGCMSaveLinPtr (pThis->pDevIns, i, linearAddr, size, iLinPtr, pCmd->paLinPtrs, &pPages);
 
                                  iLinPtr++;
                              }
@@ -911,7 +977,7 @@ int vmmdevHGCMCall (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCMCall, uint32
                              if (pPageListInfo->flags & VBOX_HGCM_F_PARM_DIRECTION_TO_HOST)
                              {
                                  /* Copy pages to the pcBuf[size]. */
-                                 rc = vmmdevHGCMPageListRead(pVMMDevState->pDevIns, pcBuf, size, pPageListInfo);
+                                 rc = vmmdevHGCMPageListRead(pThis->pDevIns, pcBuf, size, pPageListInfo);
                              }
                              else
                                  rc = VINF_SUCCESS;
@@ -990,7 +1056,7 @@ int vmmdevHGCMCall (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCMCall, uint32
                          {
                              /* Don't overdo it */
                              if (pGuestParm->type != VMMDevHGCMParmType_LinAddr_Out)
-                                 rc = PDMDevHlpPhysReadGCVirt(pVMMDevState->pDevIns, pcBuf, linearAddr, size);
+                                 rc = PDMDevHlpPhysReadGCVirt(pThis->pDevIns, pcBuf, linearAddr, size);
                              else
                                  rc = VINF_SUCCESS;
 
@@ -1002,7 +1068,7 @@ int vmmdevHGCMCall (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCMCall, uint32
                                  /* Remember the guest physical pages that belong to the virtual address region.
                                   * Do it for all linear pointers because load state will require In pointer info too.
                                   */
-                                 rc = vmmdevHGCMSaveLinPtr (pVMMDevState->pDevIns, i, linearAddr, size, iLinPtr, pCmd->paLinPtrs, &pPages);
+                                 rc = vmmdevHGCMSaveLinPtr (pThis->pDevIns, i, linearAddr, size, iLinPtr, pCmd->paLinPtrs, &pPages);
 
                                  iLinPtr++;
                              }
@@ -1052,7 +1118,7 @@ int vmmdevHGCMCall (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCMCall, uint32
                              if (pPageListInfo->flags & VBOX_HGCM_F_PARM_DIRECTION_TO_HOST)
                              {
                                  /* Copy pages to the pcBuf[size]. */
-                                 rc = vmmdevHGCMPageListRead(pVMMDevState->pDevIns, pcBuf, size, pPageListInfo);
+                                 rc = vmmdevHGCMPageListRead(pThis->pDevIns, pcBuf, size, pPageListInfo);
                              }
                              else
                                  rc = VINF_SUCCESS;
@@ -1079,16 +1145,16 @@ int vmmdevHGCMCall (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCMCall, uint32
 
     if (RT_SUCCESS (rc))
     {
-        vmmdevHGCMAddCommand (pVMMDevState, pCmd, GCPhys, pHGCMCall->header.header.size, VBOXHGCMCMDTYPE_CALL);
+        vmmdevHGCMAddCommand (pThis, pCmd, GCPhys, pHGCMCall->header.header.size, VBOXHGCMCMDTYPE_CALL);
 
         /* Pass the function call to HGCM connector for actual processing */
-        rc = pVMMDevState->pHGCMDrv->pfnCall (pVMMDevState->pHGCMDrv, pCmd, pHGCMCall->u32ClientID,
+        rc = pThis->pHGCMDrv->pfnCall (pThis->pHGCMDrv, pCmd, pHGCMCall->u32ClientID,
                                               pHGCMCall->u32Function, cParms, pCmd->paHostParms);
 
         if (RT_FAILURE (rc))
         {
             Log(("vmmdevHGCMCall: pfnCall failed rc = %Rrc\n", rc));
-            vmmdevHGCMRemoveCommand (pVMMDevState, pCmd);
+            vmmdevHGCMRemoveCommand (pThis, pCmd);
         }
     }
 
@@ -1118,7 +1184,7 @@ static void logRelLoadStateBufferSizeMismatch (uint32_t size, uint32_t iPage, ui
 }
 
 
-static int vmmdevHGCMCallSaved (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCMCall, RTGCPHYS GCPhys, uint32_t cbHGCMCall, bool f64Bits, bool *pfHGCMCalled, VBOXHGCMCMD *pSavedCmd, VBOXHGCMCMD **ppCmd)
+static int vmmdevHGCMCallSaved (PVMMDEV pThis, VMMDevHGCMCall *pHGCMCall, RTGCPHYS GCPhys, uint32_t cbHGCMCall, bool f64Bits, bool *pfHGCMCalled, VBOXHGCMCMD *pSavedCmd, VBOXHGCMCMD **ppCmd)
 {
     int rc = VINF_SUCCESS;
 
@@ -1135,6 +1201,20 @@ static int vmmdevHGCMCallSaved (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCM
     Log(("vmmdevHGCMCall: cParms = %d\n", cParms));
 
     /*
+     * Sane upper limit.
+     */
+    if (cParms > VMMDEV_MAX_HGCM_PARMS)
+    {
+        static int s_cRelWarn;
+        if (s_cRelWarn < 50)
+        {
+            s_cRelWarn++;
+            LogRel(("VMMDev: request packet with too many parameters (%d). Refusing operation.\n", cParms));
+        }
+        return VERR_INVALID_PARAMETER;
+    }
+
+    /*
      * Compute size of required memory buffer.
      */
 
@@ -1274,7 +1354,7 @@ static int vmmdevHGCMCallSaved (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCM
     vmmdevRestoreSavedCommand(pCmd, pSavedCmd);
     *ppCmd = pCmd;
 
-    vmmdevHGCMAddCommand (pVMMDevState, pCmd, GCPhys, pHGCMCall->header.header.size, VBOXHGCMCMDTYPE_CALL);
+    vmmdevHGCMAddCommand (pThis, pCmd, GCPhys, pHGCMCall->header.header.size, VBOXHGCMCMDTYPE_CALL);
 
     /* Process parameters, changing them to host context pointers for easy
      * processing by connector. Guest must insure that the pointed data is actually
@@ -1289,6 +1369,7 @@ static int vmmdevHGCMCallSaved (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCM
         uint8_t *pu8Buf = (uint8_t *)pHostParm + cParms * sizeof (VBOXHGCMSVCPARM);
 
         pCmd->paHostParms = pHostParm;
+        pCmd->cHostParms = cParms;
 
         uint32_t iParm;
         int iLinPtr = 0;
@@ -1382,7 +1463,7 @@ static int vmmdevHGCMCallSaved (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCM
                                              cbChunk = cbRemaining;
                                          }
 
-                                         rc = PDMDevHlpPhysRead(pVMMDevState->pDevIns,
+                                         rc = PDMDevHlpPhysRead(pThis->pDevIns,
                                                                 pLinPtr->paPages[iPage] + offPage,
                                                                 pu8Dst, cbChunk);
 
@@ -1451,7 +1532,7 @@ static int vmmdevHGCMCallSaved (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCM
                              if (pPageListInfo->flags & VBOX_HGCM_F_PARM_DIRECTION_TO_HOST)
                              {
                                  /* Copy pages to the pcBuf[size]. */
-                                 rc = vmmdevHGCMPageListRead(pVMMDevState->pDevIns, pu8Buf, size, pPageListInfo);
+                                 rc = vmmdevHGCMPageListRead(pThis->pDevIns, pu8Buf, size, pPageListInfo);
                              }
                              else
                                  rc = VINF_SUCCESS;
@@ -1562,7 +1643,7 @@ static int vmmdevHGCMCallSaved (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCM
                                              cbChunk = cbRemaining;
                                          }
 
-                                         rc = PDMDevHlpPhysRead(pVMMDevState->pDevIns,
+                                         rc = PDMDevHlpPhysRead(pThis->pDevIns,
                                                                 pLinPtr->paPages[iPage] + offPage,
                                                                 pu8Dst, cbChunk);
 
@@ -1631,7 +1712,7 @@ static int vmmdevHGCMCallSaved (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCM
                              if (pPageListInfo->flags & VBOX_HGCM_F_PARM_DIRECTION_TO_HOST)
                              {
                                  /* Copy pages to the pcBuf[size]. */
-                                 rc = vmmdevHGCMPageListRead(pVMMDevState->pDevIns, pu8Buf, size, pPageListInfo);
+                                 rc = vmmdevHGCMPageListRead(pThis->pDevIns, pu8Buf, size, pPageListInfo);
                              }
                              else
                                  rc = VINF_SUCCESS;
@@ -1659,11 +1740,13 @@ static int vmmdevHGCMCallSaved (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCM
     if (RT_SUCCESS (rc))
     {
         /* Pass the function call to HGCM connector for actual processing */
-        rc = pVMMDevState->pHGCMDrv->pfnCall (pVMMDevState->pHGCMDrv, pCmd, pHGCMCall->u32ClientID, pHGCMCall->u32Function, cParms, pCmd->paHostParms);
+        rc = pThis->pHGCMDrv->pfnCall (pThis->pHGCMDrv, pCmd, pHGCMCall->u32ClientID, pHGCMCall->u32Function, cParms, pCmd->paHostParms);
         if (RT_SUCCESS (rc))
         {
             *pfHGCMCalled = true;
         }
+        /* else
+         *  ... the caller will also execute vmmdevHGCMRemoveCommand() for us */
     }
 
     return rc;
@@ -1674,10 +1757,10 @@ static int vmmdevHGCMCallSaved (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCM
  *
  * @thread EMT
  */
-int vmmdevHGCMCancel (VMMDevState *pVMMDevState, VMMDevHGCMCancel *pHGCMCancel, RTGCPHYS GCPhys)
+int vmmdevHGCMCancel (PVMMDEV pThis, VMMDevHGCMCancel *pHGCMCancel, RTGCPHYS GCPhys)
 {
     NOREF(pHGCMCancel);
-    int rc = vmmdevHGCMCancel2(pVMMDevState, GCPhys);
+    int rc = vmmdevHGCMCancel2(pThis, GCPhys);
     return rc == VERR_NOT_FOUND ? VERR_INVALID_PARAMETER : rc;
 }
 
@@ -1693,7 +1776,7 @@ int vmmdevHGCMCancel (VMMDevState *pVMMDevState, VMMDevHGCMCancel *pHGCMCancel,
  *
  * @thread EMT
  */
-int vmmdevHGCMCancel2 (VMMDevState *pThis, RTGCPHYS GCPhys)
+int vmmdevHGCMCancel2 (PVMMDEV pThis, RTGCPHYS GCPhys)
 {
     if (    GCPhys == 0
         ||  GCPhys == NIL_RTGCPHYS
@@ -1758,11 +1841,91 @@ static int vmmdevHGCMCmdVerify (PVBOXHGCMCMD pCmd, VMMDevHGCMRequestHeader *pHea
     return VERR_INVALID_PARAMETER;
 }
 
-#define PDMIHGCMPORT_2_VMMDEVSTATE(pInterface) ( (VMMDevState *) ((uintptr_t)pInterface - RT_OFFSETOF(VMMDevState, IHGCMPort)) )
+#ifdef VBOX_WITH_64_BITS_GUESTS
+static int vmmdevHGCMParmVerify64(HGCMFunctionParameter64 *pGuestParm, VBOXHGCMSVCPARM *pHostParm)
+{
+    int rc = VERR_INVALID_PARAMETER;
+
+    switch (pGuestParm->type)
+    {
+        case VMMDevHGCMParmType_32bit:
+            if (pHostParm->type == VBOX_HGCM_SVC_PARM_32BIT)
+                rc = VINF_SUCCESS;
+            break;
+
+        case VMMDevHGCMParmType_64bit:
+            if (pHostParm->type == VBOX_HGCM_SVC_PARM_64BIT)
+                rc = VINF_SUCCESS;
+            break;
+
+        case VMMDevHGCMParmType_LinAddr_In:  /* In (read) */
+        case VMMDevHGCMParmType_LinAddr_Out: /* Out (write) */
+        case VMMDevHGCMParmType_LinAddr:     /* In & Out */
+            if (   pHostParm->type == VBOX_HGCM_SVC_PARM_PTR
+                && pGuestParm->u.Pointer.size >= pHostParm->u.pointer.size)
+                rc = VINF_SUCCESS;
+            break;
+
+        case VMMDevHGCMParmType_PageList:
+            if (   pHostParm->type == VBOX_HGCM_SVC_PARM_PTR
+                && pGuestParm->u.PageList.size >= pHostParm->u.pointer.size)
+                rc = VINF_SUCCESS;
+            break;
+
+        default:
+            AssertLogRelMsgFailed(("hgcmCompleted: invalid parameter type %08X\n", pGuestParm->type));
+            break;
+    }
+
+    return rc;
+}
+#endif /* VBOX_WITH_64_BITS_GUESTS */
+
+#ifdef VBOX_WITH_64_BITS_GUESTS
+static int vmmdevHGCMParmVerify32(HGCMFunctionParameter32 *pGuestParm, VBOXHGCMSVCPARM *pHostParm)
+#else
+static int vmmdevHGCMParmVerify32(HGCMFunctionParameter *pGuestParm, VBOXHGCMSVCPARM *pHostParm)
+#endif
+{
+    int rc = VERR_INVALID_PARAMETER;
+
+    switch (pGuestParm->type)
+    {
+        case VMMDevHGCMParmType_32bit:
+            if (pHostParm->type == VBOX_HGCM_SVC_PARM_32BIT)
+                rc = VINF_SUCCESS;
+            break;
+
+        case VMMDevHGCMParmType_64bit:
+            if (pHostParm->type == VBOX_HGCM_SVC_PARM_64BIT)
+                rc = VINF_SUCCESS;
+            break;
+
+        case VMMDevHGCMParmType_LinAddr_In:  /* In (read) */
+        case VMMDevHGCMParmType_LinAddr_Out: /* Out (write) */
+        case VMMDevHGCMParmType_LinAddr:     /* In & Out */
+            if (   pHostParm->type == VBOX_HGCM_SVC_PARM_PTR
+                && pGuestParm->u.Pointer.size >= pHostParm->u.pointer.size)
+                rc = VINF_SUCCESS;
+            break;
+
+        case VMMDevHGCMParmType_PageList:
+            if (   pHostParm->type == VBOX_HGCM_SVC_PARM_PTR
+                && pGuestParm->u.PageList.size >= pHostParm->u.pointer.size)
+                rc = VINF_SUCCESS;
+            break;
+
+        default:
+            AssertLogRelMsgFailed(("hgcmCompleted: invalid parameter type %08X\n", pGuestParm->type));
+            break;
+    }
+
+    return rc;
+}
 
 DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result, PVBOXHGCMCMD pCmd)
 {
-    VMMDevState *pVMMDevState = PDMIHGCMPORT_2_VMMDEVSTATE(pInterface);
+    PVMMDEV pThis = RT_FROM_MEMBER(pInterface, VMMDevState, IHGCMPort);
 #ifdef VBOX_WITH_DTRACE
     uint32_t idFunction = 0;
     uint32_t idClient   = 0;
@@ -1790,7 +1953,7 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
      * back to guest memory.
      */
     VBOXDD_HGCMCALL_COMPLETED_EMT(pCmd, result);
-    vmmdevHGCMRemoveCommand (pVMMDevState, pCmd);
+    vmmdevHGCMRemoveCommand (pThis, pCmd);
 
     if (pCmd->fCancelled || pCmd->fSaved)
     {
@@ -1830,15 +1993,14 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
          * the request. (This isn't 100% optimal, but it solves the
          * 3.0 blocker.)
          */
-        /** @todo s/pVMMDevState/pThis/g */
         /** @todo It would be faster if this interface would use MMIO2 memory and we
          *        didn't have to mess around with PDMDevHlpPhysRead/Write. We're
          *        reading the header 3 times now and writing the request back twice. */
 
-        PDMCritSectEnter(&pVMMDevState->CritSect, VERR_SEM_BUSY);
-        PDMCritSectLeave(&pVMMDevState->CritSect);
+        PDMCritSectEnter(&pThis->CritSect, VERR_SEM_BUSY);
+        PDMCritSectLeave(&pThis->CritSect);
 
-        PDMDevHlpPhysRead(pVMMDevState->pDevIns, pCmd->GCPhys, pHeader, pCmd->cbSize);
+        PDMDevHlpPhysRead(pThis->pDevIns, pCmd->GCPhys, pHeader, pCmd->cbSize);
 
         /* Setup return codes. */
         pHeader->result = result;
@@ -1858,6 +2020,8 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
                 VMMDevHGCMCall *pHGCMCall = (VMMDevHGCMCall *)pHeader;
 
                 uint32_t cParms = pHGCMCall->cParms;
+                if (cParms != pCmd->cHostParms)
+                    rc = VERR_INVALID_PARAMETER;
 
                 VBOXHGCMSVCPARM *pHostParm = pCmd->paHostParms;
 
@@ -1866,8 +2030,12 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
 
                 HGCMFunctionParameter64 *pGuestParm = VMMDEV_HGCM_CALL_PARMS64(pHGCMCall);
 
-                for (i = 0; i < cParms; i++, pGuestParm++, pHostParm++)
+                for (i = 0; i < cParms && RT_SUCCESS(rc); i++, pGuestParm++, pHostParm++)
                 {
+                    rc = vmmdevHGCMParmVerify64(pGuestParm, pHostParm);
+                    if (RT_FAILURE(rc))
+                        break;
+
                     switch (pGuestParm->type)
                     {
                         case VMMDevHGCMParmType_32bit:
@@ -1892,9 +2060,8 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
                                 if (pGuestParm->type != VMMDevHGCMParmType_LinAddr_In)
                                 {
                                     /* Use the saved page list to write data back to the guest RAM. */
-                                    rc = vmmdevHGCMWriteLinPtr (pVMMDevState->pDevIns, i, pHostParm->u.pointer.addr,
+                                    rc = vmmdevHGCMWriteLinPtr (pThis->pDevIns, i, pHostParm->u.pointer.addr,
                                                                 size, iLinPtr, pCmd->paLinPtrs);
-                                    AssertReleaseRC(rc);
                                 }
 
                                 /* All linptrs with size > 0 were saved. Advance the index to the next linptr. */
@@ -1933,7 +2100,7 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
                                 if (pPageListInfo->flags & VBOX_HGCM_F_PARM_DIRECTION_FROM_HOST)
                                 {
                                     /* Copy pHostParm->u.pointer.addr[pHostParm->u.pointer.size] to pages. */
-                                    rc = vmmdevHGCMPageListWrite(pVMMDevState->pDevIns, pPageListInfo, pHostParm->u.pointer.addr, size);
+                                    rc = vmmdevHGCMPageListWrite(pThis->pDevIns, pPageListInfo, pHostParm->u.pointer.addr, size);
                                 }
                                 else
                                     rc = VINF_SUCCESS;
@@ -1945,7 +2112,8 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
                         default:
                         {
                             /* This indicates that the guest request memory was corrupted. */
-                            AssertReleaseMsgFailed(("hgcmCompleted: invalid parameter type %08X\n", pGuestParm->type));
+                            rc = VERR_INVALID_PARAMETER;
+                            break;
                         }
                     }
                 }
@@ -1961,6 +2129,8 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
                 VMMDevHGCMCall *pHGCMCall = (VMMDevHGCMCall *)pHeader;
 
                 uint32_t cParms = pHGCMCall->cParms;
+                if (cParms != pCmd->cHostParms)
+                    rc = VERR_INVALID_PARAMETER;
 
                 VBOXHGCMSVCPARM *pHostParm = pCmd->paHostParms;
 
@@ -1969,8 +2139,12 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
 
                 HGCMFunctionParameter32 *pGuestParm = VMMDEV_HGCM_CALL_PARMS32(pHGCMCall);
 
-                for (i = 0; i < cParms; i++, pGuestParm++, pHostParm++)
+                for (i = 0; i < cParms && RT_SUCCESS(rc); i++, pGuestParm++, pHostParm++)
                 {
+                    rc = vmmdevHGCMParmVerify32(pGuestParm, pHostParm);
+                    if (RT_FAILURE(rc))
+                        break;
+
                     switch (pGuestParm->type)
                     {
                         case VMMDevHGCMParmType_32bit:
@@ -1995,8 +2169,7 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
                                 if (pGuestParm->type != VMMDevHGCMParmType_LinAddr_In)
                                 {
                                     /* Use the saved page list to write data back to the guest RAM. */
-                                    rc = vmmdevHGCMWriteLinPtr (pVMMDevState->pDevIns, i, pHostParm->u.pointer.addr, size, iLinPtr, pCmd->paLinPtrs);
-                                    AssertReleaseRC(rc);
+                                    rc = vmmdevHGCMWriteLinPtr (pThis->pDevIns, i, pHostParm->u.pointer.addr, size, iLinPtr, pCmd->paLinPtrs);
                                 }
 
                                 /* All linptrs with size > 0 were saved. Advance the index to the next linptr. */
@@ -2035,7 +2208,7 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
                                 if (pPageListInfo->flags & VBOX_HGCM_F_PARM_DIRECTION_FROM_HOST)
                                 {
                                     /* Copy pHostParm->u.pointer.addr[pHostParm->u.pointer.size] to pages. */
-                                    rc = vmmdevHGCMPageListWrite(pVMMDevState->pDevIns, pPageListInfo, pHostParm->u.pointer.addr, size);
+                                    rc = vmmdevHGCMPageListWrite(pThis->pDevIns, pPageListInfo, pHostParm->u.pointer.addr, size);
                                 }
                                 else
                                     rc = VINF_SUCCESS;
@@ -2047,7 +2220,8 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
                         default:
                         {
                             /* This indicates that the guest request memory was corrupted. */
-                            AssertReleaseMsgFailed(("hgcmCompleted: invalid parameter type %08X\n", pGuestParm->type));
+                            rc = VERR_INVALID_PARAMETER;
+                            break;
                         }
                     }
                 }
@@ -2063,6 +2237,8 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
                 VMMDevHGCMCall *pHGCMCall = (VMMDevHGCMCall *)pHeader;
 
                 uint32_t cParms = pHGCMCall->cParms;
+                if (cParms != pCmd->cHostParms)
+                    rc = VERR_INVALID_PARAMETER;
 
                 VBOXHGCMSVCPARM *pHostParm = pCmd->paHostParms;
 
@@ -2071,8 +2247,12 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
 
                 HGCMFunctionParameter *pGuestParm = VMMDEV_HGCM_CALL_PARMS(pHGCMCall);
 
-                for (i = 0; i < cParms; i++, pGuestParm++, pHostParm++)
+                for (i = 0; i < cParms && RT_SUCCESS(rc); i++, pGuestParm++, pHostParm++)
                 {
+                    rc = vmmdevHGCMParmVerify32(pGuestParm, pHostParm);
+                    if (RT_FAILURE(rc))
+                        break;
+
                     switch (pGuestParm->type)
                     {
                         case VMMDevHGCMParmType_32bit:
@@ -2097,8 +2277,7 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
                                 if (pGuestParm->type != VMMDevHGCMParmType_LinAddr_In)
                                 {
                                     /* Use the saved page list to write data back to the guest RAM. */
-                                    rc = vmmdevHGCMWriteLinPtr (pVMMDevState->pDevIns, i, pHostParm->u.pointer.addr, size, iLinPtr, pCmd->paLinPtrs);
-                                    AssertReleaseRC(rc);
+                                    rc = vmmdevHGCMWriteLinPtr (pThis->pDevIns, i, pHostParm->u.pointer.addr, size, iLinPtr, pCmd->paLinPtrs);
                                 }
 
                                 /* All linptrs with size > 0 were saved. Advance the index to the next linptr. */
@@ -2137,7 +2316,7 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
                                 if (pPageListInfo->flags & VBOX_HGCM_F_PARM_DIRECTION_FROM_HOST)
                                 {
                                     /* Copy pHostParm->u.pointer.addr[pHostParm->u.pointer.size] to pages. */
-                                    rc = vmmdevHGCMPageListWrite(pVMMDevState->pDevIns, pPageListInfo, pHostParm->u.pointer.addr, size);
+                                    rc = vmmdevHGCMPageListWrite(pThis->pDevIns, pPageListInfo, pHostParm->u.pointer.addr, size);
                                 }
                                 else
                                     rc = VINF_SUCCESS;
@@ -2149,7 +2328,8 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
                         default:
                         {
                             /* This indicates that the guest request memory was corrupted. */
-                            AssertReleaseMsgFailed(("hgcmCompleted: invalid parameter type %08X\n", pGuestParm->type));
+                            rc = VERR_INVALID_PARAMETER;
+                            break;
                         }
                     }
                 }
@@ -2175,20 +2355,21 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
                 break;
             }
         }
-        else
+
+        if (RT_FAILURE(rc))
         {
-            /* Command type is wrong. Return error to the guest. */
-            pHeader->header.rc = rc;
+            /* Command is wrong. Return HGCM error result to the guest. */
+            pHeader->result = rc;
         }
 
         /* Mark request as processed. */
         pHeader->fu32Flags |= VBOX_HGCM_REQ_DONE;
 
         /* Write back the request */
-        PDMDevHlpPhysWrite(pVMMDevState->pDevIns, pCmd->GCPhys, pHeader, pCmd->cbSize);
+        PDMDevHlpPhysWrite(pThis->pDevIns, pCmd->GCPhys, pHeader, pCmd->cbSize);
 
         /* Now, when the command was removed from the internal list, notify the guest. */
-        VMMDevNotifyGuest (pVMMDevState, VMMDEV_EVENT_HGCM);
+        VMMDevNotifyGuest (pThis, VMMDEV_EVENT_HGCM);
 
         if ((uint8_t *)pHeader != &au8Prealloc[0])
         {
@@ -2211,20 +2392,20 @@ DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result
 
 DECLCALLBACK(void) hgcmCompleted (PPDMIHGCMPORT pInterface, int32_t result, PVBOXHGCMCMD pCmd)
 {
-    VMMDevState *pVMMDevState = PDMIHGCMPORT_2_VMMDEVSTATE(pInterface);
+    PVMMDEV pThis = RT_FROM_MEMBER(pInterface, VMMDevState, IHGCMPort);
 
     VBOXDD_HGCMCALL_COMPLETED_REQ(pCmd, result);
 
 /** @todo no longer necessary to forward to EMT, but it might be more
  *        efficient...? */
     /* Not safe to execute asynchronously; forward to EMT */
-    int rc = VMR3ReqCallVoidNoWait(PDMDevHlpGetVM(pVMMDevState->pDevIns), VMCPUID_ANY,
+    int rc = VMR3ReqCallVoidNoWait(PDMDevHlpGetVM(pThis->pDevIns), VMCPUID_ANY,
                                    (PFNRT)hgcmCompletedWorker, 3, pInterface, result, pCmd);
     AssertRC(rc);
 }
 
-/* @thread EMT */
-int vmmdevHGCMSaveState(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
+/** @thread EMT */
+int vmmdevHGCMSaveState(PVMMDEV pThis, PSSMHANDLE pSSM)
 {
     /* Save information about pending requests.
      * Only GCPtrs are of interest.
@@ -2236,7 +2417,7 @@ int vmmdevHGCMSaveState(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
     /* Compute how many commands are pending. */
     uint32_t cCmds = 0;
 
-    PVBOXHGCMCMD pIter = pVMMDevState->pHGCMCmdList;
+    PVBOXHGCMCMD pIter = pThis->pHGCMCmdList;
 
     while (pIter)
     {
@@ -2253,7 +2434,7 @@ int vmmdevHGCMSaveState(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
 
     if (cCmds > 0)
     {
-        pIter = pVMMDevState->pHGCMCmdList;
+        pIter = pThis->pHGCMCmdList;
 
         while (pIter)
         {
@@ -2330,7 +2511,7 @@ int vmmdevHGCMSaveState(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
              * completed later by a still running host service.
              */
             pIter->fSaved = true;
-            vmmdevHGCMRemoveCommand (pVMMDevState, pIter);
+            vmmdevHGCMRemoveCommand (pThis, pIter);
 
             pIter = pNext;
         }
@@ -2344,7 +2525,7 @@ int vmmdevHGCMSaveState(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
 }
 
 /** @thread EMT(0) */
-int vmmdevHGCMLoadState(VMMDevState *pVMMDevState, PSSMHANDLE pSSM, uint32_t uVersion)
+int vmmdevHGCMLoadState(PVMMDEV pThis, PSSMHANDLE pSSM, uint32_t uVersion)
 {
     int rc = VINF_SUCCESS;
 
@@ -2378,7 +2559,7 @@ int vmmdevHGCMLoadState(VMMDevState *pVMMDevState, PSSMHANDLE pSSM, uint32_t uVe
 
             pCmd->enmCmdType = VBOXHGCMCMDTYPE_LOADSTATE; /* This marks the "old" saved command. */
 
-            vmmdevHGCMAddCommand (pVMMDevState, pCmd, GCPhys, cbSize, VBOXHGCMCMDTYPE_LOADSTATE);
+            vmmdevHGCMAddCommand (pThis, pCmd, GCPhys, cbSize, VBOXHGCMCMDTYPE_LOADSTATE);
         }
     }
     else
@@ -2500,7 +2681,7 @@ int vmmdevHGCMLoadState(VMMDevState *pVMMDevState, PSSMHANDLE pSSM, uint32_t uVe
             rc = SSMR3GetU32(pSSM, &u32);
             AssertRCReturn(rc, rc);
 
-            vmmdevHGCMAddCommand (pVMMDevState, pCmd, GCPhys, cbSize, VBOXHGCMCMDTYPE_LOADSTATE);
+            vmmdevHGCMAddCommand (pThis, pCmd, GCPhys, cbSize, VBOXHGCMCMDTYPE_LOADSTATE);
         }
 
         /* A reserved field, will allow to extend saved data for VMMDevHGCM. */
@@ -2511,15 +2692,15 @@ int vmmdevHGCMLoadState(VMMDevState *pVMMDevState, PSSMHANDLE pSSM, uint32_t uVe
     return rc;
 }
 
-/* @thread EMT */
-int vmmdevHGCMLoadStateDone(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
+/** @thread EMT */
+int vmmdevHGCMLoadStateDone(PVMMDEV pThis, PSSMHANDLE pSSM)
 {
     LogFlowFunc(("\n"));
 
     /* Reissue pending requests. */
-    PPDMDEVINS pDevIns = pVMMDevState->pDevIns;
+    PPDMDEVINS pDevIns = pThis->pDevIns;
 
-    int rc = vmmdevHGCMCmdListLock (pVMMDevState);
+    int rc = vmmdevHGCMCmdListLock (pThis);
 
     if (RT_SUCCESS (rc))
     {
@@ -2533,9 +2714,9 @@ int vmmdevHGCMLoadStateDone(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
          *       resubmitting the command to HGCM services.
          *       New commands will be inserted to the list.
          */
-        PVBOXHGCMCMD pIter = pVMMDevState->pHGCMCmdList;
+        PVBOXHGCMCMD pIter = pThis->pHGCMCmdList;
 
-        pVMMDevState->pHGCMCmdList = NULL; /* Reset the list. Saved commands will be processed and deallocated. */
+        pThis->pHGCMCmdList = NULL; /* Reset the list. Saved commands will be processed and deallocated. */
 
         while (pIter)
         {
@@ -2583,7 +2764,7 @@ int vmmdevHGCMLoadStateDone(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
                                 AssertMsgFailed(("VMMDevReq_HGCMConnect structure has invalid size!\n"));
                                 requestHeader->header.rc = VERR_INVALID_PARAMETER;
                             }
-                            else if (!pVMMDevState->pHGCMDrv)
+                            else if (!pThis->pHGCMDrv)
                             {
                                 Log(("VMMDevReq_HGCMConnect HGCM Connector is NULL!\n"));
                                 requestHeader->header.rc = VERR_NOT_SUPPORTED;
@@ -2594,7 +2775,7 @@ int vmmdevHGCMLoadStateDone(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
 
                                 Log(("VMMDevReq_HGCMConnect\n"));
 
-                                requestHeader->header.rc = vmmdevHGCMConnectSaved (pVMMDevState, pHGCMConnect, pIter->GCPhys, &fHGCMCalled, pIter, &pCmd);
+                                requestHeader->header.rc = vmmdevHGCMConnectSaved (pThis, pHGCMConnect, pIter->GCPhys, &fHGCMCalled, pIter, &pCmd);
                             }
                             break;
                         }
@@ -2606,7 +2787,7 @@ int vmmdevHGCMLoadStateDone(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
                                 AssertMsgFailed(("VMMDevReq_HGCMDisconnect structure has invalid size!\n"));
                                 requestHeader->header.rc = VERR_INVALID_PARAMETER;
                             }
-                            else if (!pVMMDevState->pHGCMDrv)
+                            else if (!pThis->pHGCMDrv)
                             {
                                 Log(("VMMDevReq_HGCMDisconnect HGCM Connector is NULL!\n"));
                                 requestHeader->header.rc = VERR_NOT_SUPPORTED;
@@ -2616,7 +2797,7 @@ int vmmdevHGCMLoadStateDone(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
                                 VMMDevHGCMDisconnect *pHGCMDisconnect = (VMMDevHGCMDisconnect *)requestHeader;
 
                                 Log(("VMMDevReq_VMMDevHGCMDisconnect\n"));
-                                requestHeader->header.rc = vmmdevHGCMDisconnectSaved (pVMMDevState, pHGCMDisconnect, pIter->GCPhys, &fHGCMCalled, pIter, &pCmd);
+                                requestHeader->header.rc = vmmdevHGCMDisconnectSaved (pThis, pHGCMDisconnect, pIter->GCPhys, &fHGCMCalled, pIter, &pCmd);
                             }
                             break;
                         }
@@ -2628,7 +2809,7 @@ int vmmdevHGCMLoadStateDone(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
                                 AssertMsgFailed(("VMMDevReq_HGCMCall structure has invalid size!\n"));
                                 requestHeader->header.rc = VERR_INVALID_PARAMETER;
                             }
-                            else if (!pVMMDevState->pHGCMDrv)
+                            else if (!pThis->pHGCMDrv)
                             {
                                 Log(("VMMDevReq_HGCMCall HGCM Connector is NULL!\n"));
                                 requestHeader->header.rc = VERR_NOT_SUPPORTED;
@@ -2646,7 +2827,7 @@ int vmmdevHGCMLoadStateDone(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
 #else
                                 bool f64Bits = false;
 #endif /* VBOX_WITH_64_BITS_GUESTS */
-                                requestHeader->header.rc = vmmdevHGCMCallSaved (pVMMDevState, pHGCMCall, pIter->GCPhys, requestHeader->header.size, f64Bits, &fHGCMCalled, pIter, &pCmd);
+                                requestHeader->header.rc = vmmdevHGCMCallSaved (pThis, pHGCMCall, pIter->GCPhys, requestHeader->header.size, f64Bits, &fHGCMCalled, pIter, &pCmd);
                             }
                             break;
                         }
@@ -2662,7 +2843,7 @@ int vmmdevHGCMLoadStateDone(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
                                         AssertMsgFailed(("VMMDevReq_HGCMConnect structure has invalid size!\n"));
                                         requestHeader->header.rc = VERR_INVALID_PARAMETER;
                                     }
-                                    else if (!pVMMDevState->pHGCMDrv)
+                                    else if (!pThis->pHGCMDrv)
                                     {
                                         Log(("VMMDevReq_HGCMConnect HGCM Connector is NULL!\n"));
                                         requestHeader->header.rc = VERR_NOT_SUPPORTED;
@@ -2673,7 +2854,7 @@ int vmmdevHGCMLoadStateDone(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
 
                                         Log(("VMMDevReq_HGCMConnect\n"));
 
-                                        requestHeader->header.rc = vmmdevHGCMConnect (pVMMDevState, pHGCMConnect, pIter->GCPhys);
+                                        requestHeader->header.rc = vmmdevHGCMConnect (pThis, pHGCMConnect, pIter->GCPhys);
                                     }
                                     break;
                                 }
@@ -2685,7 +2866,7 @@ int vmmdevHGCMLoadStateDone(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
                                         AssertMsgFailed(("VMMDevReq_HGCMDisconnect structure has invalid size!\n"));
                                         requestHeader->header.rc = VERR_INVALID_PARAMETER;
                                     }
-                                    else if (!pVMMDevState->pHGCMDrv)
+                                    else if (!pThis->pHGCMDrv)
                                     {
                                         Log(("VMMDevReq_HGCMDisconnect HGCM Connector is NULL!\n"));
                                         requestHeader->header.rc = VERR_NOT_SUPPORTED;
@@ -2695,7 +2876,7 @@ int vmmdevHGCMLoadStateDone(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
                                         VMMDevHGCMDisconnect *pHGCMDisconnect = (VMMDevHGCMDisconnect *)requestHeader;
 
                                         Log(("VMMDevReq_VMMDevHGCMDisconnect\n"));
-                                        requestHeader->header.rc = vmmdevHGCMDisconnect (pVMMDevState, pHGCMDisconnect, pIter->GCPhys);
+                                        requestHeader->header.rc = vmmdevHGCMDisconnect (pThis, pHGCMDisconnect, pIter->GCPhys);
                                     }
                                     break;
                                 }
@@ -2712,7 +2893,7 @@ int vmmdevHGCMLoadStateDone(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
                                         AssertMsgFailed(("VMMDevReq_HGCMCall structure has invalid size!\n"));
                                         requestHeader->header.rc = VERR_INVALID_PARAMETER;
                                     }
-                                    else if (!pVMMDevState->pHGCMDrv)
+                                    else if (!pThis->pHGCMDrv)
                                     {
                                         Log(("VMMDevReq_HGCMCall HGCM Connector is NULL!\n"));
                                         requestHeader->header.rc = VERR_NOT_SUPPORTED;
@@ -2730,7 +2911,7 @@ int vmmdevHGCMLoadStateDone(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
 #else
                                         bool f64Bits = false;
 #endif /* VBOX_WITH_64_BITS_GUESTS */
-                                        requestHeader->header.rc = vmmdevHGCMCall (pVMMDevState, pHGCMCall, requestHeader->header.size, pIter->GCPhys, f64Bits);
+                                        requestHeader->header.rc = vmmdevHGCMCall (pThis, pHGCMCall, requestHeader->header.size, pIter->GCPhys, f64Bits);
                                     }
                                     break;
                                 }
@@ -2775,7 +2956,7 @@ int vmmdevHGCMLoadStateDone(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
                    /* HGCM was not called. Deallocate the current command and then notify guest. */
                    if (pCmd)
                    {
-                       vmmdevHGCMRemoveCommand (pVMMDevState, pCmd);
+                       vmmdevHGCMRemoveCommand (pThis, pCmd);
 
                        if (pCmd->paLinPtrs != NULL)
                        {
@@ -2786,7 +2967,7 @@ int vmmdevHGCMLoadStateDone(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
                        pCmd = NULL;
                    }
 
-                   VMMDevNotifyGuest (pVMMDevState, VMMDEV_EVENT_HGCM);
+                   VMMDevNotifyGuest (pThis, VMMDEV_EVENT_HGCM);
                 }
             }
 
@@ -2801,7 +2982,7 @@ int vmmdevHGCMLoadStateDone(VMMDevState *pVMMDevState, PSSMHANDLE pSSM)
             pIter = pNext;
         }
 
-        vmmdevHGCMCmdListUnlock (pVMMDevState);
+        vmmdevHGCMCmdListUnlock (pThis);
     }
 
     return rc;