Changeset 90189 in vbox

Timestamp:

Jul 14, 2021 4:39:09 PM (4 years ago)

Author:

vboxsync

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

145709

Message:

VMM: Make the setjmp code a bit stricter with when to resume a call. bugref:10064 ticketref:20090 ticketref:20456

Location:

trunk

Files:

• include/VBox/vmm/gvm.h (modified) (1 diff)
• src/VBox/VMM/VMMR0/VMMR0.cpp (modified) (4 diffs)
• src/VBox/VMM/VMMR0/VMMR0JmpA-amd64.asm (modified) (2 diffs)
• src/VBox/VMM/VMMR3/VMMGuruMeditation.cpp (modified) (1 diff)
• src/VBox/VMM/include/VMMInternal.h (modified) (3 diffs)
• src/VBox/VMM/include/VMMInternal.mac (modified) (2 diffs)

trunk/include/VBox/vmm/gvm.h

@@ -104 +104 @@
 #endif
 
+    union
+    {
+#if defined(VMM_INCLUDED_SRC_include_VMMInternal_h) && defined(IN_RING0)
+        struct VMMR0PERVCPU s;
+#endif
+        uint8_t             padding[64];
+    } vmmr0;
+
     /** Padding the structure size to page boundrary. */
 #ifdef VBOX_WITH_NEM_R0
+    uint8_t                 abPadding2[4096 - 64 - 64 - 1024 - 64 - 64];
+#else
     uint8_t                 abPadding2[4096 - 64 - 64 - 1024 - 64];
-#else
-    uint8_t                 abPadding2[4096 - 64 - 64 - 1024];
 #endif
 } GVMCPU;

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

@@ -1678 +1678 @@
  * @remarks Assume called with interrupts _enabled_.
  */
-static int vmmR0EntryExWorker(PGVM pGVM, VMCPUID idCpu, VMMR0OPERATION enmOperation,
-                              PSUPVMMR0REQHDR pReqHdr, uint64_t u64Arg, PSUPDRVSESSION pSession)
+DECL_NO_INLINE(static, int) vmmR0EntryExWorker(PGVM pGVM, VMCPUID idCpu, VMMR0OPERATION enmOperation,
+                                               PSUPVMMR0REQHDR pReqHdr, uint64_t u64Arg, PSUPDRVSESSION pSession)
 {
     /*
@@ -2429 +2429 @@
 
 /**
- * Argument for vmmR0EntryExWrapper containing the arguments for VMMR0EntryEx.
- */
-typedef struct VMMR0ENTRYEXARGS
-{
-    PGVM                pGVM;
-    VMCPUID             idCpu;
-    VMMR0OPERATION      enmOperation;
-    PSUPVMMR0REQHDR     pReq;
-    uint64_t            u64Arg;
-    PSUPDRVSESSION      pSession;
-} VMMR0ENTRYEXARGS;
-/** Pointer to a vmmR0EntryExWrapper argument package. */
-typedef VMMR0ENTRYEXARGS *PVMMR0ENTRYEXARGS;
-
-/**
  * This is just a longjmp wrapper function for VMMR0EntryEx calls.
  *
@@ -2451 +2436 @@
 static DECLCALLBACK(int) vmmR0EntryExWrapper(void *pvArgs)
 {
-    return vmmR0EntryExWorker(((PVMMR0ENTRYEXARGS)pvArgs)->pGVM,
-                              ((PVMMR0ENTRYEXARGS)pvArgs)->idCpu,
-                              ((PVMMR0ENTRYEXARGS)pvArgs)->enmOperation,
-                              ((PVMMR0ENTRYEXARGS)pvArgs)->pReq,
-                              ((PVMMR0ENTRYEXARGS)pvArgs)->u64Arg,
-                              ((PVMMR0ENTRYEXARGS)pvArgs)->pSession);
+    PGVMCPU pGVCpu = (PGVMCPU)pvArgs;
+    return vmmR0EntryExWorker(pGVCpu->vmmr0.s.pGVM,
+                              pGVCpu->vmmr0.s.idCpu,
+                              pGVCpu->vmmr0.s.enmOperation,
+                              pGVCpu->vmmr0.s.pReq,
+                              pGVCpu->vmmr0.s.u64Arg,
+                              pGVCpu->vmmr0.s.pSession);
 }
 
@@ -2516 +2502 @@
                         break;
 
-                    /** @todo validate this EMT claim... GVM knows. */
-                    VMMR0ENTRYEXARGS Args;
-                    Args.pGVM = pGVM;
-                    Args.idCpu = idCpu;
-                    Args.enmOperation = enmOperation;
-                    Args.pReq = pReq;
-                    Args.u64Arg = u64Arg;
-                    Args.pSession = pSession;
-                    return vmmR0CallRing3SetJmpEx(&pGVCpu->vmm.s.CallRing3JmpBufR0, vmmR0EntryExWrapper, &Args);
+                    pGVCpu->vmmr0.s.pGVM         = pGVM;
+                    pGVCpu->vmmr0.s.idCpu        = idCpu;
+                    pGVCpu->vmmr0.s.enmOperation = enmOperation;
+                    pGVCpu->vmmr0.s.pReq         = pReq;
+                    pGVCpu->vmmr0.s.u64Arg       = u64Arg;
+                    pGVCpu->vmmr0.s.pSession     = pSession;
+                    return vmmR0CallRing3SetJmpEx(&pGVCpu->vmm.s.CallRing3JmpBufR0, vmmR0EntryExWrapper, pGVCpu,
+                                                  ((uintptr_t)u64Arg << 16) | (uintptr_t)enmOperation);
                 }
                 return VERR_VM_THREAD_NOT_EMT;

trunk/src/VBox/VMM/VMMR0/VMMR0JmpA-amd64.asm

@@ -128 +128 @@
     jnz     .resume
 
+.different_call_continue:
+    mov     [xDX + VMMR0JMPBUF.pfn], r11
+    mov     [xDX + VMMR0JMPBUF.pvUser1], r8
+    mov     [xDX + VMMR0JMPBUF.pvUser2], r9
+
  %ifdef VMM_R0_SWITCH_STACK
     mov     r15, [xDX + VMMR0JMPBUF.pvSavedStack]
@@ -224 +229 @@
 
     ;
+    ; Not the same call as went to ring-3.
+    ;
+.different_call:
+    mov     byte [xDX + VMMR0JMPBUF.fInRing3Call], 0
+    ;; @todo or should we fail here instead?
+    jmp     .different_call_continue
+
+    ;
     ; Resume VMMRZCallRing3 the call.
     ;
 .resume:
+    ; Check if it's actually the same call, if not just continue with it
+    ; as a regular call (ring-0 assert, then VM destroy).
+    cmp     [xDX + VMMR0JMPBUF.pfn], r11
+    jne     .different_call
+    cmp     [xDX + VMMR0JMPBUF.pvUser1], r8
+    jne     .different_call
+    cmp     [xDX + VMMR0JMPBUF.pvUser2], r9
+    jne     .different_call
+
  %ifndef VMM_R0_SWITCH_STACK
     ; Sanity checks incoming stack, applying fuzz if needed.

trunk/src/VBox/VMM/VMMR3/VMMGuruMeditation.cpp

@@ -406 +406 @@
                                 pVCpu->vmm.s.CallRing3JmpBufR0.cbUsedTotal,
                                 pVCpu->vmm.s.CallRing3JmpBufR0.cUsedTotal);
+                pHlp->pfnPrintf(pHlp,
+                                "pfn=%RHv pvUser1=%RHv pvUser2=%RHv\n",
+                                pVCpu->vmm.s.CallRing3JmpBufR0.pfn,
+                                pVCpu->vmm.s.CallRing3JmpBufR0.pvUser1,
+                                pVCpu->vmm.s.CallRing3JmpBufR0.pvUser2);
 
                 /* Dump the resume register frame on the stack. */

trunk/src/VBox/VMM/include/VMMInternal.h

@@ -164 +164 @@
     /** Unwind: The vmmR0CallRing3SetJmp return address stack location. */
     RTHCUINTREG                 UnwindRetPcLocation;
+
+    /** The function last being executed here. */
+    RTHCUINTREG                 pfn;
+    /** The first argument to the function. */
+    RTHCUINTREG                 pvUser1;
+    /** The second argument to the function. */
+    RTHCUINTREG                 pvUser2;
+
 #if HC_ARCH_BITS == 32
     /** Alignment padding. */
@@ -448 +456 @@
 typedef VMMCPU *PVMMCPU;
 
+/**
+ * VMM per-VCpu ring-0 only instance data.
+ */
+typedef struct VMMR0PERVCPU
+{
+    /** @name Arguments passed by VMMR0EntryEx via vmmR0CallRing3SetJmpEx.
+     * @note Cannot be put on the stack as the location may change and upset the
+     *       validation of resume-after-ring-3-call logic.
+     * @{ */
+    PGVM                pGVM;
+    VMCPUID             idCpu;
+    VMMR0OPERATION      enmOperation;
+    PSUPVMMR0REQHDR     pReq;
+    uint64_t            u64Arg;
+    PSUPDRVSESSION      pSession;
+    /** @} */
+} VMMR0PERVCPU;
+/** Pointer to VMM ring-0 VMCPU instance data. */
+typedef VMMR0PERVCPU *PVMMR0PERVCPU;
 
 
@@ -539 +566 @@
  * @param   pfn         The function to be called when not resuming.
  * @param   pvUser      The argument of that function.
- */
-DECLASM(int)    vmmR0CallRing3SetJmpEx(PVMMR0JMPBUF pJmpBuf, PFNVMMR0SETJMPEX pfn, void *pvUser);
+ * @param   uCallKey    Unused call parameter that should be used to help
+ *                      uniquely identify the call.
+ */
+DECLASM(int)    vmmR0CallRing3SetJmpEx(PVMMR0JMPBUF pJmpBuf, PFNVMMR0SETJMPEX pfn, void *pvUser, uintptr_t uCallKey);
 
 

trunk/src/VBox/VMM/include/VMMInternal.mac

@@ -53 +53 @@
     .UnwindRetPcValue       resd 1
     .UnwindRetPcLocation    resd 1
+    .pfn            resd 1
+    .pvUser1        resd 1
+    .pvUser2        resd 1
 %endif
 %ifdef RT_ARCH_AMD64
@@ -93 +96 @@
     .UnwindRetPcValue       resq 1
     .UnwindRetPcLocation    resq 1
+    .pfn            resq 1
+    .pvUser1        resq 1
+    .pvUser2        resq 1
 %endif