From b49a595b0ac7138242b1f616f6d5169a48b786a7 Mon Sep 17 00:00:00 2001 From: Satoshi Tanda Date: Sun, 8 Mar 2020 20:12:24 -0700 Subject: [PATCH] Fix multiple bugs, refactor InitializeMsrBitmaps --- Sources/Asm.asm | 12 +++--- Sources/HostUtils.c | 95 ++++++++++++++++++++++++++++++++++++++++---- Sources/HostUtils.h | 21 ++++++++++ Sources/HostVmcall.c | 2 +- Sources/MiniVisor.c | 21 ++++++++-- Sources/Public.h | 6 ++- 6 files changed, 139 insertions(+), 18 deletions(-) diff --git a/Sources/Asm.asm b/Sources/Asm.asm index 52966b4..e4624ae 100644 --- 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 index f3daf84..7c012e6 100644 --- 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 index a766fce..b4a7fe9 100644 --- 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 index 9fd2eb1..c96703e 100644 --- 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 index 68c349f..6e373c5 100644 --- 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 index c611b53..92482f2 100644 --- 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