From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id A64D6780091 for ; Fri, 19 Apr 2024 18:22:18 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=/lUZZHEZnVtc/G4dSVyd8JHHi6k0xEIlfYXpXyIgtbc=; c=relaxed/simple; d=groups.io; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20240206; t=1713550937; v=1; b=DJJb2sc2s86KvPr6BiFADjmPMdXSybXSwlYr2DoN1AE4VYq5CiYkVCyNl13e68lbcc9gj4dP zzjSEYhRNQsCPc2ZrU6Qt2HNXwybWrHKmkMtWr78sAl+1HQbFY8ZSIKh3K6PpH2aqEaMUOPrdgn S9obZxQO7vV/UcKrgWIGrP9ZPLWImX6bovVTSHQHEH8KvrEqmmLFq4YYvoSj8VY4DY6Z/r66p/o w+aMEGJuvYng6S4aeivk7vtRYBTMC2z9UWbrAybQw4QNnJasUWCPvJpbiahPLfSouQwtq+7yiA9 J6n2S9YEWkO9SCkYAkOe+MwLPsn53NVjJEVtGJrzhnU6g== X-Received: by 127.0.0.2 with SMTP id aIlvYY7687511x8BHapmaWI2; Fri, 19 Apr 2024 11:22:17 -0700 X-Received: from mail-pl1-f202.google.com (mail-pl1-f202.google.com [209.85.214.202]) by mx.groups.io with SMTP id smtpd.web11.1207.1713550936507079115 for ; Fri, 19 Apr 2024 11:22:16 -0700 X-Received: by mail-pl1-f202.google.com with SMTP id d9443c01a7336-1e604136b6dso28895295ad.1 for ; Fri, 19 Apr 2024 11:22:16 -0700 (PDT) X-Gm-Message-State: YqkVob6qdC0855BHJ1PIUAYxx7686176AA= X-Google-Smtp-Source: AGHT+IGT+bMqvLLKjUE/QpKMiXeJSORsFEiKOZwk8OOc0uIuT6fdW4obdw89eiR1Bdg1koD7IdSQSvyz8Iqdvw== X-Received: from anticipation.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:4517]) (user=acdunlap job=sendgmr) by 2002:a17:902:d4cc:b0:1e4:32ec:ce5d with SMTP id o12-20020a170902d4cc00b001e432ecce5dmr251604plg.0.1713550935686; Fri, 19 Apr 2024 11:22:15 -0700 (PDT) Date: Fri, 19 Apr 2024 11:21:46 -0700 In-Reply-To: Mime-Version: 1.0 References: Message-ID: <20240419182146.699506-1-acdunlap@google.com> Subject: [edk2-devel] [PATCH v2] OvmfPkg: Harden #VC instruction emulation somewhat (CVE-2024-25742) From: "Adam Dunlap via groups.io" To: devel@edk2.groups.io, Borislav Petkov , Peter Gonda , Ard Biesheuvel , Erdem Aktas , Gerd Hoffmann , Jiewen Yao , Michael Roth , Min Xu , Tom Lendacky , Yuan Yu Cc: Adam Dunlap Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Fri, 19 Apr 2024 11:22:16 -0700 Resent-From: acdunlap@google.com Reply-To: devel@edk2.groups.io,acdunlap@google.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=DJJb2sc2; dmarc=pass (policy=none) header.from=groups.io; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io 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) Cc: Tom Lendacky Signed-off-by: Adam Dunlap --- Patch changelog: V1 -> V2: Added the MWAITX/MONITORX opcodes. Added the opcode for INVD. Added a comment to explain skipping the exit handling if the opcode does not match. OvmfPkg/Library/CcExitLib/CcExitVcHandler.c | 184 ++++++++++++++++++-- 1 file changed, 173 insertions(+), 11 deletions(-) diff --git a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c index 0fc30f7bc4..31587586fe 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,170 @@ 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: + if (OpCode == 0x08) { + return 0; + } + + break; + + case SVM_EXIT_MONITOR: + CcDecodeModRm (Regs, InstructionData); + + if ((OpCode == 0x01) && + ( (InstructionData->ModRm.Uint8 == 0xc8) /* MONITOR */ + || (InstructionData->ModRm.Uint8 == 0xfa))) /* MONITORX */ + { + return 0; + } + + break; + + case SVM_EXIT_MWAIT: + CcDecodeModRm (Regs, InstructionData); + + if ((OpCode == 0x01) && + ( (InstructionData->ModRm.Uint8 == 0xc9) /* MWAIT */ + || (InstructionData->ModRm.Uint8 == 0xfb))) /* MWAITX */ + { + 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 +1927,15 @@ InternalVmgExitHandleVc ( CcInitInstructionData (&InstructionData, Ghcb, Regs); - Status = NaeExit (Ghcb, Regs, &InstructionData); + Status = VcCheckOpcodeBytes (Ghcb, Regs, &InstructionData, ExitCode); + + // + // If the opcode does not match the exit code, do not process the exception + // + if (Status == 0) { + Status = NaeExit (Ghcb, Regs, &InstructionData); + } + if (Status == 0) { Regs->Rip += CcInstructionLength (&InstructionData); } else { -- 2.44.0.769.g3c40516874-goog -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118038): https://edk2.groups.io/g/devel/message/118038 Mute This Topic: https://groups.io/mt/105623545/7686176 Mute #vc:https://edk2.groups.io/g/devel/mutehashtag/vc Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-