Changeset 31437 in vbox

Timestamp:

Aug 6, 2010 12:01:55 PM (15 years ago)

Author:

vboxsync

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

64503

Message:

patmR3HandleDirtyInstr: Don't access the record after it has been freed.

File:

• trunk/src/VBox/VMM/PATM/PATM.cpp (modified) (26 diffs)

trunk/src/VBox/VMM/PATM/PATM.cpp

@@ -138 +138 @@
      *
      */
-    Assert(sizeof(PATMGCSTATE) < PAGE_SIZE);    /** @note hardcoded dependencies on this exist. */
+    Assert(sizeof(PATMGCSTATE) < PAGE_SIZE);    /* Note: hardcoded dependencies on this exist. */
     pVM->patm.s.pGCStateHC  = (PPATMGCSTATE)((uint8_t *)pVM->patm.s.pGCStackHC + PATM_STACK_TOTAL_SIZE);
     pVM->patm.s.pGCStateGC  = MMHyperR3ToRC(pVM, pVM->patm.s.pGCStateHC);
@@ -1577 +1577 @@
         return rc;
 
-    /** @note Never do a direct return unless a failure is encountered! */
+    /* Note: Never do a direct return unless a failure is encountered! */
 
     /* Clear recompilation of next instruction flag; we are doing that right here. */
@@ -1888 +1888 @@
 
     case OP_MOV_DR:
-        /** @note: currently we let DRx writes cause a trap d; our trap handler will decide to interpret it or not. */
+        /* Note: currently we let DRx writes cause a trap d; our trap handler will decide to interpret it or not. */
         if (pCpu->pCurInstr->param2 == OP_PARM_Dd)
         {
@@ -1899 +1899 @@
 
     case OP_MOV_CR:
-        /** @note: currently we let CRx writes cause a trap d; our trap handler will decide to interpret it or not. */
+        /* Note: currently we let CRx writes cause a trap d; our trap handler will decide to interpret it or not. */
         if (pCpu->pCurInstr->param2 == OP_PARM_Cd)
         {
@@ -2345 +2345 @@
                 }
 
-                /** @note after a cli we must continue to a proper exit point */
+                /* Note: after a cli we must continue to a proper exit point */
                 if (cpunext.pCurInstr->opcode != OP_CLI)
                 {
@@ -2668 +2668 @@
     case OP_PUSHF:
         /* We can 'call' a cli or pushf patch. It will either return to the original guest code when IF is set again, or fault. */
-        /** @note special precautions are taken when disabling and enabling such patches. */
+        /* Note: special precautions are taken when disabling and enabling such patches. */
         pPatch->flags |= PATMFL_CALLABLE_AS_FUNCTION;
         break;
@@ -2717 +2717 @@
 
     /***************************************************************************************************************************/
-    /** @note We can't insert *any* code before a sysenter handler; some linux guests have an invalid stack at this point!!!!! */
+    /* Note: We can't insert *any* code before a sysenter handler; some linux guests have an invalid stack at this point!!!!!  */
     /***************************************************************************************************************************/
 #ifdef VBOX_WITH_STATISTICS
@@ -3152 +3152 @@
     cpu.mode = (pPatch->flags & PATMFL_CODE32) ? CPUMODE_32BIT : CPUMODE_16BIT;
 
-    /** @note Set the PATM interrupt flag here; it was cleared before the patched call. (!!!) */
+    /* Note: Set the PATM interrupt flag here; it was cleared before the patched call. (!!!) */
     rc = patmPatchGenSetPIF(pVM, pPatch, pInstrGC);
     if (RT_FAILURE(rc))
@@ -3303 +3303 @@
     pPatch->uCurPatchOffset   = 0;
 
-    /** @note Set the PATM interrupt flag here; it was cleared before the patched call. (!!!) */
+    /* Note: Set the PATM interrupt flag here; it was cleared before the patched call. (!!!) */
     rc = patmPatchGenSetPIF(pVM, pPatch, pInstrGC);
     if (RT_FAILURE(rc))
@@ -3775 +3775 @@
     int rc;
 
-    /** @note Do not use patch memory here! It might called during patch installation too. */
+    /* Note: Do not use patch memory here! It might called during patch installation too. */
 
 #ifdef LOG_ENABLED
@@ -4029 +4029 @@
     }
 
-    /** @note the OpenBSD specific check will break if we allow additional patches to be installed (int 3)) */
+    /* Note: the OpenBSD specific check will break if we allow additional patches to be installed (int 3)) */
     if (!(flags & PATMFL_GUEST_SPECIFIC))
     {
@@ -4036 +4036 @@
     }
 
-    /** @note obsolete */
+    /* Note: obsolete */
     if (    PATMIsPatchGCAddr(pVM, pInstrGC)
         && (flags & PATMFL_MMIO_ACCESS))
@@ -4773 +4773 @@
                             int rc = PATMR3RemovePatch(pVM, pPatch->pPrivInstrGC);
                             if (rc == VINF_SUCCESS)
-                                /** @note jump back to the start as the pPatchPage has been deleted or changed */
+                                /* Note: jump back to the start as the pPatchPage has been deleted or changed */
                                 goto loop_start;
 
@@ -4815 +4815 @@
                                     PATMR3MarkDirtyPatch(pVM, pPatch);
 
-                                    /** @note jump back to the start as the pPatchPage has been deleted or changed */
+                                    /* Note: jump back to the start as the pPatchPage has been deleted or changed */
                                     goto loop_start;
                                 }
@@ -4854 +4854 @@
                         if (pPatch->cInvalidWrites > PATM_MAX_INVALID_WRITES)
                         {
-                            /** @note possibly dangerous assumption that all future writes will be harmless. */
+                            /* Note: possibly dangerous assumption that all future writes will be harmless. */
                             if (pPatch->flags & PATMFL_IDTHANDLER)
                             {
@@ -4868 +4868 @@
                                 PATMR3MarkDirtyPatch(pVM, pPatch);
                             }
-                            /** @note jump back to the start as the pPatchPage has been deleted or changed */
+                            /* Note: jump back to the start as the pPatchPage has been deleted or changed */
                             goto invalid_write_loop_start;
                         }
@@ -5002 +5002 @@
 
         /* Clear the IDT entries for the patch we're disabling. */
-        /** @note very important as we clear IF in the patch itself */
+        /* Note: very important as we clear IF in the patch itself */
         /** @todo this needs to be changed */
         if (pPatch->flags & PATMFL_IDTHANDLER)
@@ -5363 +5363 @@
     Log(("PATMRemovePatch %RRv\n", pPatch->pPrivInstrGC));
 
-    /** @note NEVER EVER REUSE PATCH MEMORY */
-    /** @note PATMR3DisablePatch puts a breakpoint (0xCC) at the entry of this patch */
+    /* Note: NEVER EVER REUSE PATCH MEMORY */
+    /* Note: PATMR3DisablePatch puts a breakpoint (0xCC) at the entry of this patch */
 
     if (pPatchRec->patch.pPatchBlockOffset)
@@ -5403 +5403 @@
 #endif
 
-    /** @note no need to free Guest2PatchAddrTree as those records share memory with Patch2GuestAddrTree records. */
+    /* Note: no need to free Guest2PatchAddrTree as those records share memory with Patch2GuestAddrTree records. */
     patmEmptyTreeU32(pVM, &pPatch->Patch2GuestAddrTree);
     pPatch->nrPatch2GuestRecs = 0;
@@ -5415 +5415 @@
         MMR3HeapFree(pPatchRec->patch.pTempInfo);
 
-    /** @note might fail, because it has already been removed (e.g. during reset). */
+    /* Note: might fail, because it has already been removed (e.g. during reset). */
     RTAvloU32Remove(&pVM->patm.s.PatchLookupTreeHC->PatchTree, pPatchRec->Core.Key);
 
@@ -5657 +5657 @@
         return VINF_SUCCESS;
 
-    /** @note we don't restore patch pages for patches that are not enabled! */
-    /** @note be careful when changing this behaviour!! */
+    /* Note: we don't restore patch pages for patches that are not enabled! */
+    /* Note: be careful when changing this behaviour!! */
 
     /* The patch pages are no longer marked for self-modifying code detection */
@@ -5852 +5852 @@
     uint32_t     cbDirty;
     PRECPATCHTOGUEST pRec;
+    RTRCPTR const pOrgInstrGC = pPatchToGuestRec->pOrgInstrGC;
     PVMCPU       pVCpu = VMMGetCpu0(pVM);
-
-    Log(("patmR3HandleDirtyInstr: dirty instruction at %RRv (%RRv)\n", pEip, pPatchToGuestRec->pOrgInstrGC));
+    Log(("patmR3HandleDirtyInstr: dirty instruction at %RRv (%RRv)\n", pEip, pOrgInstrGC));
 
     pRec             = pPatchToGuestRec;
-    pCurInstrGC      = pPatchToGuestRec->pOrgInstrGC;
+    pCurInstrGC      = pOrgInstrGC;
     pCurPatchInstrGC = pEip;
     cbDirty          = 0;
@@ -5867 +5867 @@
         if (pRec->fJumpTarget)
         {
-            LogRel(("PATM: patmR3HandleDirtyInstr: dirty instruction at %RRv (%RRv) ignored, because instruction in function was reused as target of jump\n", pEip, pPatchToGuestRec->pOrgInstrGC));
+            LogRel(("PATM: patmR3HandleDirtyInstr: dirty instruction at %RRv (%RRv) ignored, because instruction in function was reused as target of jump\n", pEip, pOrgInstrGC));
             pRec->fDirty = false;
             return VERR_PATCHING_REFUSED;
@@ -5898 +5898 @@
         pRec->fDirty      = false;
 
-        /** Remove old lookup record. */
+        /* Remove old lookup record. */
         patmr3RemoveP2GLookupRecord(pVM, &pPatch->patch, pCurPatchInstrGC);
+        pCurPatchInstrGC = NULL;
 
         pCurPatchInstrGC += CpuOld.opsize;
@@ -5936 +5937 @@
                 RTRCPTR pTargetGC = PATMResolveBranch(&CpuNew, pCurInstrGC);
 
-                if (    pTargetGC >= pPatchToGuestRec->pOrgInstrGC
-                    &&  pTargetGC <= pPatchToGuestRec->pOrgInstrGC + cbDirty
+                if (    pTargetGC >= pOrgInstrGC
+                    &&  pTargetGC <= pOrgInstrGC + cbDirty
                    )
                 {
@@ -6087 +6088 @@
 
     /* Find the patch record. */
-    /** @note there might not be a patch to guest translation record (global function) */
+    /* Note: there might not be a patch to guest translation record (global function) */
     offset = pEip - pVM->patm.s.pPatchMemGC;
     pvPatchCoreOffset = RTAvloU32GetBestFit(&pVM->patm.s.PatchLookupTreeHC->PatchTreeByPatchAddr, offset, false);