linux-kernel - Re: [PATCH 3/7] KVM: TDX: Retry locally in TDX EPT violation handler on RET_PF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Z6MUiXxYeTvX0QFm@yzhao56-desk.sh.intel.com>
Date: Wed, 5 Feb 2025 15:34:33 +0800
From: Yan Zhao <yan.y.zhao@...el.com>
To: Sean Christopherson <seanjc@...gle.com>
CC: <pbonzini@...hat.com>, <kvm@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <rick.p.edgecombe@...el.com>,
	<kai.huang@...el.com>, <adrian.hunter@...el.com>,
	<reinette.chatre@...el.com>, <xiaoyao.li@...el.com>,
	<tony.lindgren@...el.com>, <binbin.wu@...ux.intel.com>,
	<dmatlack@...gle.com>, <isaku.yamahata@...el.com>, <isaku.yamahata@...il.com>
Subject: Re: [PATCH 3/7] KVM: TDX: Retry locally in TDX EPT violation handler
 on RET_PF_RETRY

On Mon, Jan 27, 2025 at 09:04:59AM -0800, Sean Christopherson wrote:
> On Mon, Jan 27, 2025, Yan Zhao wrote:
> > On Fri, Jan 24, 2025 at 05:23:36PM -0800, Sean Christopherson wrote:
> > My previous consideration is that
> > when there's a pending interrupt that can be recognized, given the current VM
> > Exit reason is EPT violation, the next VM Entry will not deliver the interrupt
> > since the condition to recognize and deliver interrupt is unchanged after the
> > EPT violation VM Exit.
> > So checking pending interrupt brings only false positive, which is unlike
> > checking PID that the vector in the PID could arrive after the EPT violation VM
> > Exit and PID would be cleared after VM Entry even if the interrupts are not
> > deliverable. So checking PID may lead to true positive and less false positive.
> > 
> > But I understand your point now. As checking PID can also be false positive, it
> > would be no harm to introduce another source of false positive.
> 
> Yep.  FWIW, I agree that checking VMXIP is theoretically "worse", in the sense
> that it's much more likely to be a false positive.  Off the top of my head, the
> only time VMXIP will be set with RFLAGS.IF=1 on an EPT Violation is if the EPT
> violation happens in an STI or MOV_SS/POP_SS shadow.
> 
> > So using kvm_vcpu_has_events() looks like a kind of trade-off?
> > kvm_vcpu_has_events() can make TDX's code less special but might lead to the
> > local vCPU more vulnerable to the 0-step mitigation, especially when interrupts
> > are disabled in the guest.
> 
> Ya.  I think it's worth worth using kvm_vcpu_has_events() though.  In practice,
> observing VMXIP=1 with RFLAGS=0 on an EPT Violation means the guest is accessing
> memory for the first time in atomic kernel context.  That alone seems highly
> unlikely.  Add in that triggering retry requires an uncommon race, and the overall
> probability becomes miniscule.
Ok. Will use kvm_vcpu_has_events() instead.

> 
> > > That code needs a comment, because depending on the behavior of that field, it
> > > might not even be correct.
> > > 
> > > > (2) kvm_vcpu_has_events() may lead to unnecessary breaks due to exception
> > > >     pending. However, vt_inject_exception() is NULL for TDs.
> > > 
> > > Wouldn't a pending exception be a KVM bug?
> > Hmm, yes, it should be.
> > Although kvm_vcpu_ioctl_x86_set_mce() can invoke kvm_queue_exception() to queue
> > an exception for TDs, 
> 
> I thought TDX didn't support synthetic #MCs?
Hmm, it's just an example to show kvm_queue_exception() can be invoked for a TD,
as it's not disallowed :)
Another example is kvm_arch_vcpu_ioctl_set_guest_debug() when
vcpu->arch.guest_state_protected is false for a DEBUG TD.

But as you said, they are not possible to be called during vcpu_run().

> > this should not occur while VCPU_RUN is in progress.
> 
> Not "should not", _cannot_, because they're mutually exclusive.
Right, "cannot" is the accurate term.