Changeset 87529 in vbox for trunk/src/VBox/VMM/VMMR0

Timestamp:

Feb 2, 2021 10:56:44 AM (4 years ago)

Author:

vboxsync

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

142554

Message:

VMM/HMVMX: Fixed leak in hmR0VmxPagesAllocZ. Eliminated gotos in caller.

File:

• trunk/src/VBox/VMM/VMMR0/HMVMXR0.cpp (modified) (7 diffs)

trunk/src/VBox/VMM/VMMR0/HMVMXR0.cpp

@@ -1791 +1791 @@
  *
  * @returns VBox status code.
- * @param   hMemObj         The ring-0 memory object associated with the allocation.
+ * @param   phMemObj        Where to return the handle to the allocation.
  * @param   paAllocInfo     The pointer to the first element of the VMX
  *                          page-allocation info object array.
  * @param   cEntries        The number of elements in the @a paAllocInfo array.
  */
-static int hmR0VmxPagesAllocZ(RTR0MEMOBJ hMemObj, PVMXPAGEALLOCINFO paAllocInfo, uint32_t cEntries)
-{
+static int hmR0VmxPagesAllocZ(PRTR0MEMOBJ phMemObj, PVMXPAGEALLOCINFO paAllocInfo, uint32_t cEntries)
+{
+    *phMemObj = NIL_RTR0MEMOBJ;
+
     /* Figure out how many pages to allocate. */
     uint32_t cPages = 0;
@@ -1806 +1808 @@
     if (cPages)
     {
-        size_t const cbPages = cPages << X86_PAGE_4K_SHIFT;
-        int rc = RTR0MemObjAllocPage(&hMemObj, cbPages, false /* fExecutable */);
+        size_t const cbPages = cPages << PAGE_SHIFT;
+        int rc = RTR0MemObjAllocPage(phMemObj, cbPages, false /* fExecutable */);
         if (RT_FAILURE(rc))
             return rc;
 
         /* Zero the contents and assign each page to the corresponding VMX page-allocation entry. */
-        void *pvFirstPage = RTR0MemObjAddress(hMemObj);
-        ASMMemZero32(pvFirstPage, cbPages);
+        void *pvFirstPage = RTR0MemObjAddress(*phMemObj);
+        RT_BZERO(pvFirstPage, cbPages);
 
         uint32_t iPage = 0;
@@ -1819 +1821 @@
             if (paAllocInfo[i].fValid)
             {
-                RTHCPHYS const HCPhysPage = RTR0MemObjGetPagePhysAddr(hMemObj, iPage);
+                RTHCPHYS const HCPhysPage = RTR0MemObjGetPagePhysAddr(*phMemObj, iPage);
                 void          *pvPage     = (void *)((uintptr_t)pvFirstPage + (iPage << X86_PAGE_4K_SHIFT));
                 Assert(HCPhysPage && HCPhysPage != NIL_RTHCPHYS);
@@ -1914 +1916 @@
     bool const fShadowVmcs = !fIsNstGstVmcs ? pVM->hm.s.vmx.fUseVmcsShadowing : pVM->cpum.ro.GuestFeatures.fVmxVmcsShadowing;
     Assert(!pVM->cpum.ro.GuestFeatures.fVmxVmcsShadowing);  /* VMCS shadowing is not yet exposed to the guest. */
-    VMXPAGEALLOCINFO aAllocInfo[] = {
+    VMXPAGEALLOCINFO aAllocInfo[] =
+    {
         { true,        0 /* Unused */, &pVmcsInfo->HCPhysVmcs,         &pVmcsInfo->pvVmcs         },
         { true,        0 /* Unused */, &pVmcsInfo->HCPhysGuestMsrLoad, &pVmcsInfo->pvGuestMsrLoad },
@@ -1922 +1925 @@
     };
 
-    int rc = hmR0VmxPagesAllocZ(pVmcsInfo->hMemObj, &aAllocInfo[0], RT_ELEMENTS(aAllocInfo));
+    int rc = hmR0VmxPagesAllocZ(&pVmcsInfo->hMemObj, &aAllocInfo[0], RT_ELEMENTS(aAllocInfo));
     if (RT_FAILURE(rc))
         return rc;
@@ -2022 +2025 @@
     bool const fVirtApicAccess   = RT_BOOL(pVM->hm.s.vmx.Msrs.ProcCtls2.n.allowed1 & VMX_PROC_CTLS2_VIRT_APIC_ACCESS);
     bool const fUseVmcsShadowing = pVM->hm.s.vmx.fUseVmcsShadowing;
-    VMXPAGEALLOCINFO aAllocInfo[] = {
+    VMXPAGEALLOCINFO aAllocInfo[] =
+    {
         { fVirtApicAccess,   0 /* Unused */, &pVM->hm.s.vmx.HCPhysApicAccess,    (PRTR0PTR)&pVM->hm.s.vmx.pbApicAccess },
         { fUseVmcsShadowing, 0 /* Unused */, &pVM->hm.s.vmx.HCPhysVmreadBitmap,  &pVM->hm.s.vmx.pvVmreadBitmap         },
@@ -2031 +2035 @@
     };
 
-    int rc = hmR0VmxPagesAllocZ(pVM->hm.s.vmx.hMemObj, &aAllocInfo[0], RT_ELEMENTS(aAllocInfo));
-    if (RT_FAILURE(rc))
-        goto cleanup;
-
+    int rc = hmR0VmxPagesAllocZ(&pVM->hm.s.vmx.hMemObj, &aAllocInfo[0], RT_ELEMENTS(aAllocInfo));
+    if (RT_SUCCESS(rc))
+    {
 #ifdef VBOX_WITH_NESTED_HWVIRT_VMX
-    /* Allocate the shadow VMCS-fields array. */
-    if (fUseVmcsShadowing)
-    {
-        Assert(!pVM->hm.s.vmx.cShadowVmcsFields);
-        Assert(!pVM->hm.s.vmx.cShadowVmcsRoFields);
-        pVM->hm.s.vmx.paShadowVmcsFields   = (uint32_t *)RTMemAllocZ(sizeof(g_aVmcsFields));
-        pVM->hm.s.vmx.paShadowVmcsRoFields = (uint32_t *)RTMemAllocZ(sizeof(g_aVmcsFields));
-        if (RT_LIKELY(   pVM->hm.s.vmx.paShadowVmcsFields
-                      && pVM->hm.s.vmx.paShadowVmcsRoFields))
-        { /* likely */ }
-        else
-        {
-            rc = VERR_NO_MEMORY;
-            goto cleanup;
-        }
-    }
+        /* Allocate the shadow VMCS-fields array. */
+        if (fUseVmcsShadowing)
+        {
+            Assert(!pVM->hm.s.vmx.cShadowVmcsFields);
+            Assert(!pVM->hm.s.vmx.cShadowVmcsRoFields);
+            pVM->hm.s.vmx.paShadowVmcsFields   = (uint32_t *)RTMemAllocZ(sizeof(g_aVmcsFields));
+            pVM->hm.s.vmx.paShadowVmcsRoFields = (uint32_t *)RTMemAllocZ(sizeof(g_aVmcsFields));
+            if (!pVM->hm.s.vmx.paShadowVmcsFields || !pVM->hm.s.vmx.paShadowVmcsRoFields)
+                rc = VERR_NO_MEMORY;
+        }
 #endif
 
-    /*
-     * Allocate per-VCPU VT-x structures.
-     */
-    for (VMCPUID idCpu = 0; idCpu < pVM->cCpus; idCpu++)
-    {
-        /* Allocate the guest VMCS structures. */
-        PVMCPUCC pVCpu = VMCC_GET_CPU(pVM, idCpu);
-        rc = hmR0VmxAllocVmcsInfo(pVCpu, &pVCpu->hmr0.s.vmx.VmcsInfo, false /* fIsNstGstVmcs */);
-        if (RT_FAILURE(rc))
-            goto cleanup;
+        /*
+         * Allocate per-VCPU VT-x structures.
+         */
+        for (VMCPUID idCpu = 0; idCpu < pVM->cCpus && RT_SUCCESS(rc); idCpu++)
+        {
+            /* Allocate the guest VMCS structures. */
+            PVMCPUCC pVCpu = VMCC_GET_CPU(pVM, idCpu);
+            rc = hmR0VmxAllocVmcsInfo(pVCpu, &pVCpu->hmr0.s.vmx.VmcsInfo, false /* fIsNstGstVmcs */);
 
 #ifdef VBOX_WITH_NESTED_HWVIRT_VMX
-        /* Allocate the nested-guest VMCS structures, when the VMX feature is exposed to the guest. */
-        if (pVM->cpum.ro.GuestFeatures.fVmx)
-        {
-            rc = hmR0VmxAllocVmcsInfo(pVCpu, &pVCpu->hmr0.s.vmx.VmcsInfoNstGst, true /* fIsNstGstVmcs */);
-            if (RT_FAILURE(rc))
-                goto cleanup;
-        }
+            /* Allocate the nested-guest VMCS structures, when the VMX feature is exposed to the guest. */
+            if (pVM->cpum.ro.GuestFeatures.fVmx && RT_SUCCESS(rc))
+                rc = hmR0VmxAllocVmcsInfo(pVCpu, &pVCpu->hmr0.s.vmx.VmcsInfoNstGst, true /* fIsNstGstVmcs */);
 #endif
-    }
-
-    return VINF_SUCCESS;
-
-cleanup:
+        }
+        if (RT_SUCCESS(rc))
+            return VINF_SUCCESS;
+    }
     hmR0VmxStructsFree(pVM);
-    Assert(rc != VINF_SUCCESS);
     return rc;
 }