Fix multiple bugs, refactor InitializeMsrBitmaps

tandasat · Mar 9, 2020 · b49a595 · b49a595
1 parent d19ea35
commit b49a595
Show file tree

Hide file tree

Showing 6 changed files with 139 additions and 18 deletions.
diff --git a/Sources/Asm.asm b/Sources/Asm.asm
@@ -123,12 +123,12 @@ AsmHypervisorEntryPoint proc frame
         ;
         ; Restore XMM registers.
         ;
-        movaps  xmm0, xmmword ptr [rsp +  0h]
-        movaps  xmm1, xmmword ptr [rsp + 10h]
-        movaps  xmm2, xmmword ptr [rsp + 20h]
-        movaps  xmm3, xmmword ptr [rsp + 30h]
-        movaps  xmm4, xmmword ptr [rsp + 40h]
         movaps  xmm5, xmmword ptr [rsp + 50h]
+        movaps  xmm4, xmmword ptr [rsp + 40h]
+        movaps  xmm3, xmmword ptr [rsp + 30h]
+        movaps  xmm2, xmmword ptr [rsp + 20h]
+        movaps  xmm1, xmmword ptr [rsp + 10h]
+        movaps  xmm0, xmmword ptr [rsp +  0h]
         add     rsp, 68h
 
         ;
@@ -182,7 +182,7 @@ VmxError:
         ; VMRESUME or VMXOFF instruction failed. Unrecoverble. The most useful
         ; thing to do here is to call a C-function to diagnose the issue.
         ;
-        pushf
+        pushfq
         PUSHAQ
         mov     rcx, rsp
         sub     rsp, 20h

diff --git a/Sources/HostUtils.c b/Sources/HostUtils.c
@@ -421,13 +421,16 @@ FindImageBase (
     UINT64 GuestVirtualAddress
     )
 {
+    UINT64 imageBase;
+
     //
-    // Starting with the 1MB aligned address, and search up IMAGE_DOS_SIGNATURE
-    // every 1MB.
+    // Starting with the page aligned address, and search up IMAGE_DOS_SIGNATURE
+    // every page up to 16MB (0x1000000). Ntoskrnl.exe can be mapped at the page
+    // boundary and not the 64KB boundary unlike other images.
     //
-    for (UINT64 imageBase = (GuestVirtualAddress & ~(0x10000 - 1));
-         /**/;
-         imageBase -= 0x10000)
+    imageBase = (GuestVirtualAddress & ~(PAGE_SIZE - 1));
+
+    for (int i = 0; i < 0x1000; i++, imageBase -= PAGE_SIZE)
     {
         BOOLEAN ok;
         UINT16 contents;
@@ -441,12 +444,90 @@ FindImageBase (
                                      &errorInfo);
         if (ok == FALSE)
         {
-            return 0;
+            continue;
         }
 
         if (contents == 0x5A4D)
         {
-            return imageBase;
+            goto Exit;
         }
     }
+
+    imageBase = 0;
+
+Exit:
+    return imageBase;
+}
+
+_Use_decl_annotations_
+VOID
+UpdateMsrBitmaps (
+    MSR_BITMAPS* Bitmaps,
+    IA32_MSR_ADDRESS Msr,
+    OPERATION_TYPE InterOperation,
+    BOOLEAN Intercept
+    )
+{
+    IA32_MSR_ADDRESS msrTemp;
+    BOOLEAN highValue;
+    UINT64 byteOffset;
+    UINT64 bitMask;
+    UINT8* msrBitmap;
+
+    //
+    // MSR must be within either 0x0 - 0x1fff or 0xc0000000 - 0xc0001fff
+    // inclusive, and at least read or write intercept must be specified.
+    //
+    MV_ASSERT((Msr <= 0x1fff) ||
+              ((Msr >= 0xc0000000) && (Msr <= 0xc0001fff)));
+
+    //
+    // Check if the MSR belongs to high bitmaps.
+    //
+    highValue = BooleanFlagOn(Msr, 0xc0000000);
+
+    //
+    // Computes offsets and bitmaps to update the bitmaps.
+    //
+    msrTemp = (Msr & ~0xc0000000);
+    byteOffset = (msrTemp / CHAR_BIT);
+    bitMask = (1ull << (msrTemp % CHAR_BIT));
+
+    //
+    // Select the bitmap to work on.
+    //
+    if (InterOperation == OperationRead)
+    {
+        if (highValue == FALSE)
+        {
+            msrBitmap = Bitmaps->ReadBitmapLow;
+        }
+        else
+        {
+            msrBitmap = Bitmaps->ReadBitmapHigh;
+        }
+    }
+    else
+    {
+        if (highValue == FALSE)
+        {
+            msrBitmap = Bitmaps->WriteBitmapLow;
+        }
+        else
+        {
+            msrBitmap = Bitmaps->WriteBitmapHigh;
+        }
+    }
+
+    //
+    // Set of clear the bit.
+    //
+    if (Intercept != FALSE)
+    {
+        SetFlag(msrBitmap[byteOffset], bitMask);
+    }
+    else
+    {
+        ClearFlag(msrBitmap[byteOffset], bitMask);
+    }
 }
diff --git a/Sources/HostUtils.h b/Sources/HostUtils.h
@@ -231,3 +231,24 @@ FindImageBase (
     _In_ GUEST_CONTEXT* GuestContext,
     _In_ UINT64 GuestVirtualAddress
     );
+
+/*!
+    @brief Updates the MSR bitmap as specified.
+
+    @param[in] Bitmaps - The pointer to the MSR bitmaps.
+
+    @param[in,out] Msr - The MSR to change configurations. Must be in the range of
+        0x0 - 0x1fff or 0xc0000000 - 0xc0001fff.
+
+    @param[in] InterOperation - The type of operation to change configurations.
+
+    @param[in] Intercept - TRUE if the hypervisor should intercept the specified
+        type of access.
+ */
+VOID
+UpdateMsrBitmaps (
+    _Inout_ MSR_BITMAPS* Bitmaps,
+    _In_ IA32_MSR_ADDRESS Msr,
+    _In_ OPERATION_TYPE InterOperation,
+    _In_ BOOLEAN Intercept
+    );
diff --git a/Sources/HostVmcall.c b/Sources/HostVmcall.c
@@ -43,7 +43,7 @@ HandleVmcallUninstall (
     //
     gdtr.BaseAddress = VmxRead(VMCS_GUEST_GDTR_BASE);
     gdtr.Limit = (UINT16)VmxRead(VMCS_GUEST_GDTR_LIMIT);
-    _sgdt(&gdtr);
+    _lgdt(&gdtr);
 
     idtr.BaseAddress = VmxRead(VMCS_GUEST_IDTR_BASE);
     idtr.Limit = (UINT16)VmxRead(VMCS_GUEST_IDTR_LIMIT);

diff --git a/Sources/MiniVisor.c b/Sources/MiniVisor.c
@@ -996,6 +996,7 @@ SetupVmcs (
     // Finally, place necessary data for hypervisor into the hypervisor stack.
     //
     ProcessorContext->HypervisorStack.u.Layout.Context.ProcessorNumber = GetCurrentProcessorNumber();
+    ProcessorContext->HypervisorStack.u.Layout.Context.SharedMsrBitmaps = &SharedProcessorContext->MsrBitmaps;
     ProcessorContext->HypervisorStack.u.Layout.Context.SharedProcessorContext = SharedProcessorContext;
     ProcessorContext->HypervisorStack.u.Layout.Context.EptContext = &ProcessorContext->EptContext;
     ProcessorContext->HypervisorStack.u.Layout.Context.MemoryAccessContext = &ProcessorContext->MemoryAccessContext;
@@ -1178,14 +1179,28 @@ InitializeMsrBitmaps (
     _Out_ MSR_BITMAPS* Bitmaps
     )
 {
-    static CONST UINT64 biosSignatureByteOffset = (IA32_BIOS_SIGN_ID / CHAR_BIT);
-    static CONST UINT64 biosSignatureBitMask = (1ull << (IA32_BIOS_SIGN_ID % CHAR_BIT));
+    typedef struct _INTERCEPT_MSR_REGISTRATION
+    {
+        IA32_MSR_ADDRESS Msr;
+        OPERATION_TYPE InterceptType;
+    } INTERCEPT_MSR_REGISTRATION;
+
+    static CONST INTERCEPT_MSR_REGISTRATION registrations[] =
+    {
+        { IA32_BIOS_SIGN_ID, OperationRead, },
+    };
 
     PAGED_CODE();
 
     RtlZeroMemory(Bitmaps, sizeof(*Bitmaps));
 
-    Bitmaps->ReadBitmapLow[biosSignatureByteOffset] |= biosSignatureBitMask;
+    for (int i = 0; i < RTL_NUMBER_OF(registrations); ++i)
+    {
+        UpdateMsrBitmaps(Bitmaps,
+                         registrations[i].Msr,
+                         registrations[i].InterceptType,
+                         TRUE);
+    }
 }
 
 /*!

diff --git a/Sources/Public.h b/Sources/Public.h
@@ -41,7 +41,11 @@ typedef struct _HYPERVISOR_CONTEXT
     //
     UINT32 ProcessorNumber;
     UINT32 Padding1;
-    UINT64 Padding2;
+
+    //
+    // The pointer to the MSR bitmaps that are shared across processor.
+    //
+    struct _MSR_BITMAPS* SharedMsrBitmaps;
 
     //
     // A pointer to the shared processor context. This value is not used by the