On 4/17/24 11:54, Adam Dunlap wrote:
Ensure that when a #VC exception happens, the instruction at the
instruction pointer matches the instruction that is expected given the
error code. This is to mitigate the ahoi WeSee attack [1] that could
allow hypervisors to breach integrity and confidentiality of the
firmware by maliciously injecting interrupts. This change is a
translated version of a linux patch e3ef461af35a ("x86/sev: Harden #VC
instruction emulation somewhat")
[1] https://ahoi-attacks.github.io/wesee/
Cc: Borislav Petkov (AMD) <b...@alien8.de>
Cc: Tom Lendacky <thomas.lenda...@amd.com>
Signed-off-by: Adam Dunlap <acdun...@google.com>
---
OvmfPkg/Library/CcExitLib/CcExitVcHandler.c | 171 ++++++++++++++++++--
1 file changed, 160 insertions(+), 11 deletions(-)
diff --git a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
index 0fc30f7bc4..bd3e9f304a 100644
--- a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
+++ b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
@@ -532,8 +532,6 @@ MwaitExit (
IN CC_INSTRUCTION_DATA *InstructionData
)
{
- CcDecodeModRm (Regs, InstructionData);
-
Ghcb->SaveArea.Rax = Regs->Rax;
CcExitVmgSetOffsetValid (Ghcb, GhcbRax);
Ghcb->SaveArea.Rcx = Regs->Rcx;
@@ -564,8 +562,6 @@ MonitorExit (
IN CC_INSTRUCTION_DATA *InstructionData
)
{
- CcDecodeModRm (Regs, InstructionData);
-
Ghcb->SaveArea.Rax = Regs->Rax; // Identity mapped, so VA = PA
CcExitVmgSetOffsetValid (Ghcb, GhcbRax);
Ghcb->SaveArea.Rcx = Regs->Rcx;
@@ -670,8 +666,6 @@ VmmCallExit (
{
UINT64 Status;
- CcDecodeModRm (Regs, InstructionData);
-
Ghcb->SaveArea.Rax = Regs->Rax;
CcExitVmgSetOffsetValid (Ghcb, GhcbRax);
Ghcb->SaveArea.Cpl = (UINT8)(Regs->Cs & 0x3);
@@ -1603,8 +1597,6 @@ Dr7WriteExit (
Ext = &InstructionData->Ext;
SevEsData = (SEV_ES_PER_CPU_DATA *)(Ghcb + 1);
- CcDecodeModRm (Regs, InstructionData);
-
//
// MOV DRn always treats MOD == 3 no matter how encoded
//
@@ -1655,8 +1647,6 @@ Dr7ReadExit (
Ext = &InstructionData->Ext;
SevEsData = (SEV_ES_PER_CPU_DATA *)(Ghcb + 1);
- CcDecodeModRm (Regs, InstructionData);
-
//
// MOV DRn always treats MOD == 3 no matter how encoded
//
@@ -1671,6 +1661,160 @@ Dr7ReadExit (
return 0;
}
+/**
+ Check that the opcode matches the exit code for a #VC.
+
+ Each exit code should only be raised while executing certain instructions.
+ Verify that rIP points to a correct instruction based on the exit code to
+ protect against maliciously injected interrupts via the hypervisor. If it
does
+ not, report an unsupported event to the hypervisor.
+
+ Decodes the ModRm byte into InstructionData if necessary.
+
+ @param[in, out] Ghcb Pointer to the Guest-Hypervisor
Communication
+ Block
+ @param[in, out] Regs x64 processor context
+ @param[in, out] InstructionData Instruction parsing context
+ @param[in] ExitCode Exit code given by #VC.
+
+ @retval 0 No problems detected.
+ @return New exception value to propagate
+
+
+**/
+STATIC
+UINT64
+VcCheckOpcodeBytes (
+ IN OUT GHCB *Ghcb,
+ IN OUT EFI_SYSTEM_CONTEXT_X64 *Regs,
+ IN OUT CC_INSTRUCTION_DATA *InstructionData,
+ IN UINT64 ExitCode
+ )
+{
+ UINT8 OpCode;
+
+ //
+ // Expected opcodes are either 1 or 2 bytes. If they are 2 bytes, they always
+ // start with TWO_BYTE_OPCODE_ESCAPE (0x0f), so skip over that.
+ //
+ OpCode = *(InstructionData->OpCodes);
+ if (OpCode == TWO_BYTE_OPCODE_ESCAPE) {
+ OpCode = *(InstructionData->OpCodes + 1);
+ }
+
+ switch (ExitCode) {
+ case SVM_EXIT_IOIO_PROT:
+ case SVM_EXIT_NPF:
+ /* handled separately */
+ return 0;
+
+ case SVM_EXIT_CPUID:
+ if (OpCode == 0xa2) {
+ return 0;
+ }
+
+ break;
+
+ case SVM_EXIT_INVD:
+ break;
This changes the current behavior today, but I'm ok with that.
+
+ case SVM_EXIT_MONITOR:
+ CcDecodeModRm (Regs, InstructionData);
+
+ if ((OpCode == 0x01) && (InstructionData->ModRm.Uint8 == 0xc8)) {
This should also handle the MONITORX opcode (hmmm... I need to send a
patch to the kernel).
+ return 0;
+ }
+
+ break;
+
+ case SVM_EXIT_MWAIT:
+ CcDecodeModRm (Regs, InstructionData);
+
+ if ((OpCode == 0x01) && (InstructionData->ModRm.Uint8 == 0xc9)) {
Same here for MWAITX.
Thanks,
Tom
+ return 0;
+ }
+
+ break;
+
+ case SVM_EXIT_MSR:
+ /* RDMSR */
+ if ((OpCode == 0x32) ||
+ /* WRMSR */
+ (OpCode == 0x30))
+ {
+ return 0;
+ }
+
+ break;
+
+ case SVM_EXIT_RDPMC:
+ if (OpCode == 0x33) {
+ return 0;
+ }
+
+ break;
+
+ case SVM_EXIT_RDTSC:
+ if (OpCode == 0x31) {
+ return 0;
+ }
+
+ break;
+
+ case SVM_EXIT_RDTSCP:
+ CcDecodeModRm (Regs, InstructionData);
+
+ if ((OpCode == 0x01) && (InstructionData->ModRm.Uint8 == 0xf9)) {
+ return 0;
+ }
+
+ break;
+
+ case SVM_EXIT_DR7_READ:
+ CcDecodeModRm (Regs, InstructionData);
+
+ if ((OpCode == 0x21) &&
+ (InstructionData->Ext.ModRm.Reg == 7))
+ {
+ return 0;
+ }
+
+ break;
+
+ case SVM_EXIT_VMMCALL:
+ CcDecodeModRm (Regs, InstructionData);
+
+ if ((OpCode == 0x01) && (InstructionData->ModRm.Uint8 == 0xd9)) {
+ return 0;
+ }
+
+ break;
+
+ case SVM_EXIT_DR7_WRITE:
+ CcDecodeModRm (Regs, InstructionData);
+
+ if ((OpCode == 0x23) &&
+ (InstructionData->Ext.ModRm.Reg == 7))
+ {
+ return 0;
+ }
+
+ break;
+
+ case SVM_EXIT_WBINVD:
+ if (OpCode == 0x9) {
+ return 0;
+ }
+
+ break;
+
+ default:
+ break;
+ }
+
+ return UnsupportedExit (Ghcb, Regs, InstructionData);
+}
+
/**
Handle a #VC exception.
@@ -1773,7 +1917,12 @@ InternalVmgExitHandleVc (
CcInitInstructionData (&InstructionData, Ghcb, Regs);
- Status = NaeExit (Ghcb, Regs, &InstructionData);
+ Status = VcCheckOpcodeBytes (Ghcb, Regs, &InstructionData, ExitCode);
+
+ if (Status == 0) {
+ Status = NaeExit (Ghcb, Regs, &InstructionData);
+ }
+
if (Status == 0) {
Regs->Rip += CcInstructionLength (&InstructionData);
} else {
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118031): https://edk2.groups.io/g/devel/message/118031
Mute This Topic: https://groups.io/mt/105581633/21656
Mute #vc:https://edk2.groups.io/g/devel/mutehashtag/vc
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
