Changeset 98574 in vbox

Timestamp:

Feb 15, 2023 12:24:25 AM (2 years ago)

Author:

vboxsync

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

155860

Message:

HGCMMock.h,tstGuestControlMockHGCM: Fixed memory overrun wrt client memory. Quick fix for use-after-free wrt call parameters (may still leak stuff because the code seems a bit uncertain about who does what). Disabled some non-working copy-paste tests.

Location:

trunk

Files:

• include/VBox/GuestHost/HGCMMock.h (modified) (11 diffs)
• src/VBox/HostServices/GuestControl/testcase/tstGuestControlMockHGCM.cpp (modified) (11 diffs)
• src/VBox/HostServices/SharedClipboard/testcase/tstClipboardMockHGCM.cpp (modified) (1 diff)

trunk/include/VBox/GuestHost/HGCMMock.h

@@ -171 +171 @@
      *  0 is considered as being invalid. */
     HGCMCLIENTID       uNextClientId;
-    /** Size (in bytes) of opaque pvClient area to reserve
-     *  for a connected client. */
-    size_t             cbClient;
     /** Array of connected HGCM mock clients.
      *  Currently limited to 4 clients maximum. */
@@ -198 +195 @@
 
 /** Static HGCM service to mock. */
-static TSTHGCMMOCKSVC s_tstHgcmSvc;
+static TSTHGCMMOCKSVC g_tstHgcmSvc;
+
 
 /*********************************************************************************************************************************
@@ -206 +204 @@
 PTSTHGCMMOCKCLIENT TstHgcmMockSvcWaitForConnectEx(PTSTHGCMMOCKSVC pSvc, RTMSINTERVAL msTimeout);
 PTSTHGCMMOCKCLIENT TstHgcmMockSvcWaitForConnect(PTSTHGCMMOCKSVC pSvc);
-int                TstHgcmMockSvcCreate(PTSTHGCMMOCKSVC pSvc, size_t cbClient);
+int                TstHgcmMockSvcCreate(PTSTHGCMMOCKSVC pSvc);
 int                TstHgcmMockSvcDestroy(PTSTHGCMMOCKSVC pSvc);
 int                TstHgcmMockSvcStart(PTSTHGCMMOCKSVC pSvc);
@@ -281 +279 @@
     PTSTHGCMMOCKCLIENT pClient = &pSvc->aHgcmClient[pSvc->uNextClientId];
 
-    int rc = tstHgcmMockClientInit(pClient, pSvc->uNextClientId, pSvc->cbClient);
+    int rc = tstHgcmMockClientInit(pClient, pSvc->uNextClientId, pSvc->fnTable.cbClient);
     if (RT_FAILURE(rc))
         return rc;
@@ -349 +347 @@
     pFn->u.Call.hCall    = callHandle;
     pFn->u.Call.iFunc    = function;
-    pFn->u.Call.pParms   = (PVBOXHGCMSVCPARM)RTMemDup(paParms, cbParms);
+    PVBOXHGCMSVCPARM const paParmsCopy = (PVBOXHGCMSVCPARM)RTMemDup(paParms, cbParms);
+    pFn->u.Call.pParms   = paParmsCopy;
     AssertPtrReturn(pFn->u.Call.pParms, VERR_NO_MEMORY);
     pFn->u.Call.cParms   = cParms;
@@ -361 +360 @@
     AssertRCReturn(rc2, rc2);
 
-    memcpy(paParms, pFn->u.Call.pParms, cbParms);
+    memcpy(paParms, paParmsCopy, cbParms);
+    /** @todo  paParmsCopy is leaked, right? Doesn't appear to be a
+     *         use-after-free here. (pFn is freeded though) */
 
     return VINF_SUCCESS; /** @todo Return host call rc */
@@ -503 +504 @@
                     case TSTHGCMMOCKFNTYPE_CALL:
                     {
-                        pSvc->fnTable.pfnCall(NULL, pFn->u.Call.hCall, pFn->pClient->idClient, pFn->pClient->pvClient,
-                                              pFn->u.Call.iFunc, pFn->u.Call.cParms, pFn->u.Call.pParms, RTTimeMilliTS());
+                        pSvc->fnTable.pfnCall(pSvc->fnTable.pvService, pFn->u.Call.hCall, pFn->pClient->idClient,
+                                              pFn->pClient->pvClient, pFn->u.Call.iFunc, pFn->u.Call.cParms,
+                                              pFn->u.Call.pParms, RTTimeNanoTS());
 
                         /* Note: Call will be completed in the call completion callback. */
@@ -512 +514 @@
                     case TSTHGCMMOCKFNTYPE_HOST_CALL:
                     {
-                        pSvc->rcHostCall = pSvc->fnTable.pfnHostCall(NULL, pFn->u.HostCall.iFunc, pFn->u.HostCall.cParms, pFn->u.HostCall.pParms);
+                        pSvc->rcHostCall = pSvc->fnTable.pfnHostCall(pSvc->fnTable.pvService, pFn->u.HostCall.iFunc,
+                                                                     pFn->u.HostCall.cParms, pFn->u.HostCall.pParms);
 
                         int rc2 = RTSemEventSignal(pSvc->hEventHostCall);
@@ -544 +547 @@
 PTSTHGCMMOCKSVC TstHgcmMockSvcInst(void)
 {
-    return &s_tstHgcmSvc;
+    return &g_tstHgcmSvc;
 }
 
@@ -581 +584 @@
  * @return VBox status code.
  * @param  pSvc                 HGCM mock service instance to create.
- * @param  cbClient             Size (in bytes) of service-specific client data to
- *                              allocate for a HGCM mock client.
- */
-int TstHgcmMockSvcCreate(PTSTHGCMMOCKSVC pSvc, size_t cbClient)
-{
-    AssertReturn(cbClient, VERR_INVALID_PARAMETER);
-
+ */
+int TstHgcmMockSvcCreate(PTSTHGCMMOCKSVC pSvc)
+{
     RT_ZERO(pSvc->aHgcmClient);
     pSvc->fShutdown = false;
@@ -600 +599 @@
             {
                 RTListInit(&pSvc->lstCall);
-
-                pSvc->cbClient = cbClient;
             }
         }

trunk/src/VBox/HostServices/GuestControl/testcase/tstGuestControlMockHGCM.cpp

@@ -26 +26 @@
  */
 
+
+/*********************************************************************************************************************************
+*   Header Files                                                                                                                 *
+*********************************************************************************************************************************/
 #include <VBox/HostServices/GuestControlSvc.h>
 #include <VBox/VBoxGuestLib.h>
@@ -51 +55 @@
 *   Shared Clipboard testing                                                                                                     *
 *********************************************************************************************************************************/
+/** @todo r=bird: Clipboard?  */
+
 struct CLIPBOARDTESTDESC;
 /** Pointer to a test description. */
@@ -103 +109 @@
 CLIPBOARDTESTCTX g_TstCtx;
 
+#if 0 /** @todo r=bird: Clipboard? This times out and asserts and doesn't seems to do anything sensible. */
+
 /**
  * Structure for keeping a clipboard test description.
@@ -120 +128 @@
 } SHCLCONTEXT;
 
+#endif
+
 
 static void testGuestSimple(void)
@@ -155 +165 @@
  * Test: Guest reading from host                                                                                                 *
  ********************************************************************************************************************************/
+/** @todo r=bird: Reading from the host? WTF?  Doesn't seem to work, so I've disabled it till it can be rewritten and
+ * made to do something useful rather than asserting. */
+#if 0
 typedef struct TSTUSERMOCK
 {
@@ -193 +206 @@
 {
     RT_NOREF(pTstCtx, ppvCtx);
-
-    int rc = VINF_SUCCESS;
-
-    return rc;
+    return VINF_SUCCESS;
 }
 
@@ -233 +243 @@
 }
 
+#endif
+
 
 /*********************************************************************************************************************************
  * Main                                                                                                                          *
  ********************************************************************************************************************************/
+#if 0 /** @todo r=bird: Same as above.  */
 
 /** Test definition table. */
@@ -265 +278 @@
 }
 
-int main(int argc, char *argv[])
+#endif
+
+int main()
 {
     /*
      * Init the runtime, test and say hello.
      */
-    const char *pcszExecName;
-    NOREF(argc);
-    pcszExecName = strrchr(argv[0], '/');
-    pcszExecName = pcszExecName ? pcszExecName + 1 : argv[0];
-    RTEXITCODE rcExit = RTTestInitAndCreate(pcszExecName, &g_hTest);
+    RTEXITCODE rcExit = RTTestInitAndCreate("tstGuestControlMockHGCM", &g_hTest);
     if (rcExit != RTEXITCODE_SUCCESS)
         return rcExit;
     RTTestBanner(g_hTest);
 
-#ifndef DEBUG_andy
+#if 0 //ndef DEBUG_andy - bird: fix the 'ing code.
     /* Don't let assertions in the host service panic (core dump) the test cases. */
     RTAssertSetMayPanic(false);
@@ -285 +296 @@
 
     PTSTHGCMMOCKSVC const pSvc = TstHgcmMockSvcInst();
-
-    TstHgcmMockSvcCreate(pSvc, 42 /** @todo */);
+    TstHgcmMockSvcCreate(pSvc);
     TstHgcmMockSvcStart(pSvc);
 
@@ -298 +308 @@
     }
 
+#if 0 /** @todo r=bird: Clipboard? This times out and asserts and doesn't seems to do anything sensible. */
     RT_ZERO(g_TstCtx);
 
@@ -311 +322 @@
 
     TstHGCMUtilsTaskDestroy(pTask);
+#endif
 
     TstHgcmMockSvcStop(pSvc);

trunk/src/VBox/HostServices/SharedClipboard/testcase/tstClipboardMockHGCM.cpp

@@ -699 +699 @@
 
     PTSTHGCMMOCKSVC const pSvc = TstHgcmMockSvcInst();
-    TstHgcmMockSvcCreate(pSvc, sizeof(SHCLCLIENT));
+    TstHgcmMockSvcCreate(pSvc);
     TstHgcmMockSvcStart(pSvc);