On 20.06.24 18:34, Mathias Krause wrote:
> attached patch makes the read-only GDT handling CET compatible
>
> Quoting the patch description:
> """
> CET enabled systems need to disable CR4.CET prior to attempting to
> toggle CR0.WP to avoid triggering a #GP(0). This is needed in VMMR0.r0
> for PaX's r/o GDT handling.
>
> Enhance the r/o GDT handling to toggle CR4.CET as well in case it's enabled.
Testing unveiled that interrupts need to be disabled around toggling
CR4.CET to not trip up sanity checks related to Linux's per-cpu CR4 shadow.
Attached is an updated version that takes this into account and tries to
merge interrupt flag restore with the restore_fs path.
Thanks,
Mathias
>
> This patch is provided under the MIT license.
> """
>
> It would be nice to get it integrated not only in trunk, but all still
> maintained release branches as well.
>
> Thanks,
> Mathias
CET enabled systems need to disable CR4.CET prior to attempting to toggle
CR0.WP to avoid triggering a #GP(0). This is needed in VMMR0.r0 for PaX's
r/o GDT handling.
Enhance the r/o GDT handling to toggle CR4.CET as well in case it's enabled.
This patch is provided under the MIT license.
Index: src/VBox/VMM/VMMR0/HMR0A.asm
===================================================================
--- src/VBox/VMM/VMMR0/HMR0A.asm (revision 104448)
+++ src/VBox/VMM/VMMR0/HMR0A.asm (working copy)
@@ -405,6 +405,7 @@
pushfq
cli ; (see above)
+.restore_fs_after_cli:
test edi, VMX_RESTORE_HOST_CAN_USE_WRFSBASE_AND_WRGSBASE
jz .restore_fs_using_wrmsr
@@ -440,6 +441,8 @@
.gdt_readonly_or_need_writable:
test edi, VMX_RESTORE_HOST_GDT_NEED_WRITABLE
jnz .gdt_readonly_need_writable
+ test edi, VMX_RESTORE_HOST_CET_ENABLED
+ jnz .gdt_readonly_cet
.gdt_readonly:
mov rcx, cr0
mov r9, rcx
@@ -451,6 +454,36 @@
mov cr0, r9
jmp .restore_fs
+.gdt_readonly_cet:
+ ;
+ ; CR4.CET, when enabled, needs to be disabled around toggling CR0.WP to
+ ; not #GP(0).
+ ;
+ ; We need to disable interrupts while doing so to ensure the kernel
+ ; sees a consistent state, i.e. no intermediate values of CR4 which
+ ; will be in conflict with the CR4 shadow.
+ ;
+ pushfq
+ cli
+ mov rcx, cr4
+ mov r8, rcx
+ and rcx, ~X86_CR4_CET
+ mov cr4, rcx
+ mov rcx, cr0
+ mov r9, rcx
+ add rax, qword [rsi + VMXRESTOREHOST.HostGdtr + 2] ; xAX <- descriptor offset + GDTR.pGdt.
+ and rcx, ~X86_CR0_WP
+ mov cr0, rcx
+ and dword [rax + 4], ~RT_BIT(9) ; clear the busy flag in TSS desc (bits 0-7=base, bit 9=busy bit)
+ ltr dx
+ mov cr0, r9
+ mov cr4, r8
+ ;
+ ; Keep in sync with test at .restore_fs!
+ test edi, VMX_RESTORE_HOST_SEL_FS | VMX_RESTORE_HOST_SEL_GS
+ jnz .restore_fs_after_cli
+ jmp .restore_flags
+
ALIGNCODE(8)
.gdt_readonly_need_writable:
add rax, qword [rsi + VMXRESTOREHOST.HostGdtrRw + 2] ; xAX <- descriptor offset + GDTR.pGdtRw
Index: src/VBox/VMM/VMMR0/HMVMXR0.cpp
===================================================================
--- src/VBox/VMM/VMMR0/HMVMXR0.cpp (revision 104448)
+++ src/VBox/VMM/VMMR0/HMVMXR0.cpp (working copy)
@@ -3547,7 +3547,12 @@
/* If the host has made GDT read-only, we would need to temporarily toggle CR0.WP before writing the GDT. */
if (g_fHmHostKernelFeatures & SUPKERNELFEATURES_GDT_READ_ONLY)
+ {
fRestoreHostFlags |= VMX_RESTORE_HOST_GDT_READ_ONLY;
+ /* If CET is enabled, it needs to be disabled around toggling CR0.WP. */
+ if (uHostCr4 & X86_CR4_CET)
+ fRestoreHostFlags |= VMX_RESTORE_HOST_CET_ENABLED;
+ }
if (g_fHmHostKernelFeatures & SUPKERNELFEATURES_GDT_NEED_WRITABLE)
{
/* The GDT is read-only but the writable GDT is available. */
Index: src/VBox/VMM/include/HMInternal.h
===================================================================
--- src/VBox/VMM/include/HMInternal.h (revision 104448)
+++ src/VBox/VMM/include/HMInternal.h (working copy)
@@ -677,6 +677,7 @@
#define VMX_RESTORE_HOST_GDT_READ_ONLY RT_BIT(7)
#define VMX_RESTORE_HOST_GDT_NEED_WRITABLE RT_BIT(8)
#define VMX_RESTORE_HOST_CAN_USE_WRFSBASE_AND_WRGSBASE RT_BIT(9)
+#define VMX_RESTORE_HOST_CET_ENABLED RT_BIT(10)
/**
* This _must_ be the top most bit, so that we can easily check that it and
* something else is set w/o having to do two checks like this:
@@ -689,7 +690,7 @@
* if (pVCpu->hm.s.vmx.fRestoreHostFlags > VMX_RESTORE_HOST_REQUIRED)
* @endcode
*/
-#define VMX_RESTORE_HOST_REQUIRED RT_BIT(10)
+#define VMX_RESTORE_HOST_REQUIRED RT_BIT(11)
/** @} */
/**
Index: src/VBox/VMM/include/HMInternal.mac
===================================================================
--- src/VBox/VMM/include/HMInternal.mac (revision 104448)
+++ src/VBox/VMM/include/HMInternal.mac (working copy)
@@ -111,7 +111,8 @@
%define VMX_RESTORE_HOST_GDT_READ_ONLY 0080h ;RT_BIT(7)
%define VMX_RESTORE_HOST_GDT_NEED_WRITABLE 0100h ;RT_BIT(8)
%define VMX_RESTORE_HOST_CAN_USE_WRFSBASE_AND_WRGSBASE 0200h ;RT_BIT(9)
-%define VMX_RESTORE_HOST_REQUIRED 0400h ;RT_BIT(10) - must be the highest bit!
+%define VMX_RESTORE_HOST_CET_ENABLED 0400h ;RT_BIT(10)
+%define VMX_RESTORE_HOST_REQUIRED 0800h ;RT_BIT(11) - must be the highest bit!
struc VMXRESTOREHOST
.uHostSelDS resw 1
.uHostSelES resw 1
_______________________________________________
vbox-dev mailing list
vbox-dev@virtualbox.org
https://www.virtualbox.org/mailman/listinfo/vbox-dev
