* Re: [PATCH 1/1] OvmfPkg/VmgExitLib: HALT on #VE when access to private memory
2022-10-28 7:02 ` [PATCH 1/1] OvmfPkg/VmgExitLib: HALT on #VE when access to private memory Min Xu
@ 2022-10-28 7:14 ` Yao, Jiewen
0 siblings, 0 replies; 2+ messages in thread
From: Yao, Jiewen @ 2022-10-28 7:14 UTC (permalink / raw)
To: Xu, Min M, devel@edk2.groups.io
Cc: Aktas, Erdem, Gerd Hoffmann, James Bottomley, Tom Lendacky
Thanks for the patch.
1) I suggest to say "EPT-violation #VE on private memory is not allowed!"
2) The VMM is not trusted. We need put an unconditional CpuDeadLoop() after TDVMCALL_HALT.
3) Please test both shared address and private address in EPT-violation case.
Thank you
Yao Jiewen
> + DEBUG ((DEBUG_ERROR, "EPT-violation #VE on private memory is a bug
> or an attack."));
> + ASSERT (FALSE);
> + TdVmCall (TDVMCALL_HALT, 0, 0, 0, 0, 0);
> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Friday, October 28, 2022 3:02 PM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Aktas, Erdem
> <erdemaktas@google.com>; Gerd Hoffmann <kraxel@redhat.com>; James
> Bottomley <jejb@linux.ibm.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Tom Lendacky <thomas.lendacky@amd.com>
> Subject: [PATCH 1/1] OvmfPkg/VmgExitLib: HALT on #VE when access to
> private memory
>
> From: Min M Xu <min.m.xu@intel.com>
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4125
>
> EPT-violation #VE should be always on shared memory, which means the
> shared bit of the GuestPA should be set. But in current #VE Handler
> it is not checked. When it occurs, stop TD immediately and log out
> the error.
>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
> .../Library/VmgExitLib/VmTdExitVeHandler.c | 40 ++++++++++++++-----
> 1 file changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/OvmfPkg/Library/VmgExitLib/VmTdExitVeHandler.c
> b/OvmfPkg/Library/VmgExitLib/VmTdExitVeHandler.c
> index b73e877c093b..5bc0e9b3aa74 100644
> --- a/OvmfPkg/Library/VmgExitLib/VmTdExitVeHandler.c
> +++ b/OvmfPkg/Library/VmgExitLib/VmTdExitVeHandler.c
> @@ -300,23 +300,41 @@ MmioExit (
> IN TDCALL_VEINFO_RETURN_DATA *Veinfo
> )
> {
> - UINT64 Status;
> - UINT32 MmioSize;
> - UINT32 RegSize;
> - UINT8 OpCode;
> - BOOLEAN SeenRex;
> - UINT64 *Reg;
> - UINT8 *Rip;
> - UINT64 Val;
> - UINT32 OpSize;
> - MODRM ModRm;
> - REX Rex;
> + UINT64 Status;
> + UINT32 MmioSize;
> + UINT32 RegSize;
> + UINT8 OpCode;
> + BOOLEAN SeenRex;
> + UINT64 *Reg;
> + UINT8 *Rip;
> + UINT64 Val;
> + UINT32 OpSize;
> + MODRM ModRm;
> + REX Rex;
> + TD_RETURN_DATA TdReturnData;
> + UINT8 Gpaw;
> + UINT64 TdSharedPageMask;
>
> Rip = (UINT8 *)Regs->Rip;
> Val = 0;
> Rex.Val = 0;
> SeenRex = FALSE;
>
> + Status = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData);
> + if (Status == TDX_EXIT_REASON_SUCCESS) {
> + Gpaw = (UINT8)(TdReturnData.TdInfo.Gpaw & 0x3f);
> + TdSharedPageMask = 1ULL << (Gpaw - 1);
> + } else {
> + DEBUG ((DEBUG_ERROR, "TDCALL failed with status=%llx\n", Status));
> + return Status;
> + }
> +
> + if ((Veinfo->GuestPA & TdSharedPageMask) == 0) {
> + DEBUG ((DEBUG_ERROR, "EPT-violation #VE on private memory is a bug
> or an attack."));
> + ASSERT (FALSE);
> + TdVmCall (TDVMCALL_HALT, 0, 0, 0, 0, 0);
> + }
> +
> //
> // Default to 32bit transfer
> //
> --
> 2.29.2.windows.2
^ permalink raw reply [flat|nested] 2+ messages in thread