A few more OS layer separations. Fix unload bug when SimpleVisor is not present.

If SimpleVisor got unloaded, __cpuidex might not return valid data in
EAX:EBX. Handle this case.

6 files changed, 97 insertions, 88 deletions

diff --git a/ntint.h b/ntint.h
index a21f6e4..2837d2b 100644
--- a/ntint.h
+++ b/ntint.h
@@ -22,25 +22,6 @@ Environment:
 
 #pragma once
 
-#define DPL_USER            3
-#define DPL_SYSTEM          0
-#define KGDT64_NULL         0x00
-#define KGDT64_R0_CODE      0x10
-#define KGDT64_R0_DATA      0x18
-#define KGDT64_R3_CMCODE    0x20
-#define KGDT64_R3_DATA      0x28
-#define KGDT64_R3_CODE      0x30
-#define KGDT64_SYS_TSS      0x40
-#define KGDT64_R3_CMTEB     0x50
-#define KGDT64_R0_LDT       0x60
-#define MSR_GS_BASE         0xC0000101
-#define MSR_DEBUG_CTL       0x1D9
-#define RPL_MASK            3
-#define MTRR_TYPE_WB        6
-#define EFLAGS_ALIGN_CHECK  0x40000
-#define PAGE_SIZE           4096
-#define KERNEL_STACK_SIZE   24 * 1024
-
 #define VOID                void
 #define DECLSPEC_ALIGN(x)   __declspec(align(x))
 #define DECLSPEC_NORETURN   __declspec(noreturn)
@@ -51,50 +32,8 @@ Environment:
 #define FALSE               0
 #define UNREFERENCED_PARAMETER(x)   (x)
 
-typedef struct _KDESCRIPTOR
-{
-    UINT16 Pad[3];
-    UINT16 Limit;
-    void* Base;
-} KDESCRIPTOR, *PKDESCRIPTOR;
-typedef union _KGDTENTRY64
-{
-    struct
-    {
-        UINT16 LimitLow;
-        UINT16 BaseLow;
-        union
-        {
-            struct
-            {
-                UINT8 BaseMiddle;
-                UINT8 Flags1;
-                UINT8 Flags2;
-                UINT8 BaseHigh;
-            } Bytes;
-            struct 
-            {
-                UINT32 BaseMiddle : 8;
-                UINT32 Type : 5;
-                UINT32 Dpl : 2;
-                UINT32 Present : 1;
-                UINT32 LimitHigh : 4;
-                UINT32 System : 1;
-                UINT32 LongMode : 1;
-                UINT32 DefaultBig : 1;
-                UINT32 Granularity : 1;
-                UINT32 BaseHigh : 8;
-            } Bits;
-        };
-        UINT32 BaseUpper;
-        UINT32 MustBeZero;
-    };
-    struct
-    {
-        INT64 DataLow;
-        INT64 DataHigh;
-    };
-} KGDTENTRY64, *PKGDTENTRY64;
+#define PAGE_SIZE           4096
+#define KERNEL_STACK_SIZE   24 * 1024
 
 typedef struct DECLSPEC_ALIGN(16) _M128A
 {
diff --git a/shv.h b/shv.h
index 677c556..65a521f 100644
--- a/shv.h
+++ b/shv.h
@@ -27,7 +27,6 @@ Environment:
 #include <intrin.h>
 #include <basetsd.h>
 #include "ntint.h"
-#include "vmx.h"
 #include "shv_x.h"
 
 typedef struct _SHV_SPECIAL_REGISTERS
@@ -91,12 +90,6 @@ ShvVmxEntry (
     );
 
 VOID
-ShvVmxCleanup (
-    _In_ UINT16 Data,
-    _In_ UINT16 Teb
-    );
-
-VOID
 _sldt (
     _In_ PUINT16 Ldtr
     );
diff --git a/shv_x.h b/shv_x.h
index f0bfa0d..6fed366 100644
--- a/shv_x.h
+++ b/shv_x.h
@@ -23,6 +23,8 @@ Environment:
 
 #pragma once
 
+#include "vmx.h"
+
 #define SHV_STATUS_SUCCESS          0
 #define SHV_STATUS_NOT_AVAILABLE    -1
 #define SHV_STATUS_NO_RESOURCES     -2
diff --git a/shvos.c b/shvos.c
index a1a68cb..0dac126 100644
--- a/shvos.c
+++ b/shvos.c
@@ -65,6 +65,15 @@ typedef struct _SHV_DPC_CONTEXT
     PSHV_CALLBACK_CONTEXT Context;
 } SHV_DPC_CONTEXT, *PSHV_DPC_CONTEXT;
 
+#define KGDT64_R3_DATA      0x28
+#define KGDT64_R3_CMTEB     0x50
+
+VOID
+ShvVmxCleanup (
+    _In_ UINT16 Data,
+    _In_ UINT16 Teb
+    );
+
 NTSTATUS
 FORCEINLINE
 ShvOsErrorToError (
@@ -110,6 +119,24 @@ ShvOsDpcRoutine (
     dpcContext->Routine(dpcContext->Context);
 
     //
+    // During unload SimpleVisor uses the RtlRestoreContext function which will
+    // unfortunately use the "iretq" opcode in order to restore execution back.
+    // This causes the processor to remove the RPL bits off the segments. As
+    // the x64 kernel does not expect kernel-mode code to change the value of
+    // any segments, this results in the DS and ES segments being stuck 0x20,
+    // and the FS segment being stuck at 0x50, until the next context switch.
+    //
+    // If the DPC happened to have interrupted either the idle thread or system
+    // thread, that's perfectly fine (albeit unusual). If the DPC interrupted a
+    // 64-bit long-mode thread, that's also fine. However if the DPC interrupts
+    // a thread in compatibility-mode, running as part of WoW64, it will hit a
+    // GPF instantenously and crash.
+    //
+    // Thus, set the segments to their correct value, one more time, as a fix.
+    //
+    ShvVmxCleanup(KGDT64_R3_DATA | RPL_MASK, KGDT64_R3_CMTEB | RPL_MASK);
+
+    //
     // Wait for all DPCs to synchronize at this point
     //
     KeSignalCallDpcSynchronize(SystemArgument2);
diff --git a/shvvp.c b/shvvp.c
index 8de228e..7dd961b 100644
--- a/shvvp.c
+++ b/shvvp.c
@@ -153,28 +153,18 @@ ShvVpUnloadCallback (
     // Send the magic shutdown instruction sequence. It will return in EAX:EBX
     // the VP data for the current CPU, which we must free.
     //
+    cpuInfo[0] = cpuInfo[1] = 0;
     __cpuidex(cpuInfo, 0x41414141, 0x42424242);
-    vpData = (PSHV_VP_DATA)((UINT64)cpuInfo[0] << 32 | cpuInfo[1]);
-    ShvOsFreeContiguousAlignedMemory(vpData);
 
     //
-    // The processor will return here after the hypervisor issues a VMXOFF
-    // instruction and restores the CPU context to this location. Unfortunately
-    // because this is done with RtlRestoreContext which returns using "iretq",
-    // this causes the processor to remove the RPL bits off the segments. As
-    // the x64 kernel does not expect kernel-mode code to change the value of
-    // any segments, this results in the DS and ES segments being stuck 0x20,
-    // and the FS segment being stuck at 0x50, until the next context switch.
-    //
-    // If the DPC happened to have interrupted either the idle thread or system
-    // thread, that's perfectly fine (albeit unusual). If the DPC interrupted a
-    // 64-bit long-mode thread, that's also fine. However if the DPC interrupts
-    // a thread in compatibility-mode, running as part of WoW64, it will hit a
-    // GPF instantenously and crash.
+    // If SimpleVisor is disabled for some reason, CPUID won't return anything
+    // so don't free any memory. It will unfortunately end up leaked.
     //
-    // Thus, set the segments to their correct value, one more time, as a fix.
-    //
-    ShvVmxCleanup(KGDT64_R3_DATA | RPL_MASK, KGDT64_R3_CMTEB | RPL_MASK);
+    vpData = (PSHV_VP_DATA)((UINT64)cpuInfo[0] << 32 | cpuInfo[1]);
+    if (vpData != NULL)
+    {
+        ShvOsFreeContiguousAlignedMemory(vpData);
+    }
 }
 
 PSHV_VP_DATA
diff --git a/vmx.h b/vmx.h
index d25be3c..13d9ff6 100644
--- a/vmx.h
+++ b/vmx.h
@@ -21,6 +21,62 @@ Environment:
 --*/
 
 #pragma once
+#pragma warning(disable:4201)
+#pragma warning(disable:4214)
+
+#define DPL_USER            3
+#define DPL_SYSTEM          0
+#define MSR_GS_BASE         0xC0000101
+#define MSR_DEBUG_CTL       0x1D9
+#define RPL_MASK            3
+#define MTRR_TYPE_WB        6
+#define EFLAGS_ALIGN_CHECK  0x40000
+
+typedef struct _KDESCRIPTOR
+{
+    UINT16 Pad[3];
+    UINT16 Limit;
+    void* Base;
+} KDESCRIPTOR, *PKDESCRIPTOR;
+
+typedef union _KGDTENTRY64
+{
+    struct
+    {
+        UINT16 LimitLow;
+        UINT16 BaseLow;
+        union
+        {
+            struct
+            {
+                UINT8 BaseMiddle;
+                UINT8 Flags1;
+                UINT8 Flags2;
+                UINT8 BaseHigh;
+            } Bytes;
+            struct
+            {
+                UINT32 BaseMiddle : 8;
+                UINT32 Type : 5;
+                UINT32 Dpl : 2;
+                UINT32 Present : 1;
+                UINT32 LimitHigh : 4;
+                UINT32 System : 1;
+                UINT32 LongMode : 1;
+                UINT32 DefaultBig : 1;
+                UINT32 Granularity : 1;
+                UINT32 BaseHigh : 8;
+            } Bits;
+        };
+        UINT32 BaseUpper;
+        UINT32 MustBeZero;
+    };
+    struct
+    {
+        INT64 DataLow;
+        INT64 DataHigh;
+    };
+} KGDTENTRY64, *PKGDTENTRY64;
 
 #define CPU_BASED_VIRTUAL_INTR_PENDING          0x00000004
 #define CPU_BASED_USE_TSC_OFFSETING             0x00000008
@@ -459,3 +515,5 @@ C_ASSERT(sizeof(VMX_EPML4E) == sizeof(UINT64));
 
 #define PML4E_ENTRY_COUNT 512
 #define PDPTE_ENTRY_COUNT 512
+
+