Re: [edk2-devel] [PATCH v2] OvmfPkg: Harden #VC instruction emulation somewhat (CVE-2024-25742)

2024-04-24 Thread Ard Biesheuvel 
On Tue, 23 Apr 2024 at 11:28, Gerd Hoffmann  wrote:
>
> On Fri, Apr 19, 2024 at 11:21:46AM -0700, 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) 
> > Cc: Tom Lendacky 
> > Signed-off-by: Adam Dunlap 
>
> Reviewed-by: Gerd Hoffmann 
>

Thanks all, I've merged this now.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118221): https://edk2.groups.io/g/devel/message/118221
Mute This Topic: https://groups.io/mt/105623545/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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2] OvmfPkg: Harden #VC instruction emulation somewhat (CVE-2024-25742)

2024-04-23 Thread Gerd Hoffmann 
On Fri, Apr 19, 2024 at 11:21:46AM -0700, 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) 
> Cc: Tom Lendacky 
> Signed-off-by: Adam Dunlap 

Reviewed-by: Gerd Hoffmann 

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118132): https://edk2.groups.io/g/devel/message/118132
Mute This Topic: https://groups.io/mt/105623545/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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2] OvmfPkg: Harden #VC instruction emulation somewhat (CVE-2024-25742)

2024-04-22 Thread Lendacky, Thomas via groups.io 

On 4/19/24 13:21, Adam Dunlap via groups.io 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) 
Cc: Tom Lendacky 
Signed-off-by: Adam Dunlap 


Reviewed-by: Tom Lendacky 


---

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 0No 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))
+  {
+

[edk2-devel] [PATCH v2] OvmfPkg: Harden #VC instruction emulation somewhat (CVE-2024-25742)

2024-04-19 Thread Adam Dunlap via 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 0No 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;
+  }
+