答复: [PATCH] KVM: x86: disable pv eoi if guest gives a wrong address

["Li,Rongqing" <lirongqing@baidu.com>]

> -----邮件原件-----
> 发件人: Vitaly Kuznetsov <vkuznets@redhat.com>
> 发送时间: 2021年11月5日 17:08
> 收件人: Li,Rongqing <lirongqing@baidu.com>
> 抄送: kvm@vger.kernel.org; pbonzini@redhat.com; seanjc@google.com;
> Li,Rongqing <lirongqing@baidu.com>
> 主题: Re: [PATCH] KVM: x86: disable pv eoi if guest gives a wrong address
> 
> Li RongQing <lirongqing@baidu.com> writes:
> 
> > disable pv eoi if guest gives a wrong address, this can reduces the
> > attacked possibility for a malicious guest, and can avoid unnecessary
> > write/read pv eoi memory
> >
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > ---
> >  arch/x86/kvm/lapic.c |    9 ++++++++-
> >  1 files changed, 8 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index
> > b1de23e..0f37a8d 100644
> > --- a/arch/x86/kvm/lapic.c
> > +++ b/arch/x86/kvm/lapic.c
> > @@ -2853,6 +2853,7 @@ int kvm_lapic_enable_pv_eoi(struct kvm_vcpu
> *vcpu, u64 data, unsigned long len)
> >  	u64 addr = data & ~KVM_MSR_ENABLED;
> >  	struct gfn_to_hva_cache *ghc = &vcpu->arch.pv_eoi.data;
> >  	unsigned long new_len;
> > +	int ret;
> >
> >  	if (!IS_ALIGNED(addr, 4))
> >  		return 1;
> > @@ -2866,7 +2867,13 @@ int kvm_lapic_enable_pv_eoi(struct kvm_vcpu
> *vcpu, u64 data, unsigned long len)
> >  	else
> >  		new_len = len;
> >
> > -	return kvm_gfn_to_hva_cache_init(vcpu->kvm, ghc, addr, new_len);
> > +	ret = kvm_gfn_to_hva_cache_init(vcpu->kvm, ghc, addr, new_len);
> > +
> > +	if (ret && (vcpu->arch.pv_eoi.msr_val & KVM_MSR_ENABLED)) {
> > +		vcpu->arch.pv_eoi.msr_val &= ~KVM_MSR_ENABLED;
> > +		pr_warn_once("Disabled PV EOI during wrong address\n");
> 
> Personally, I see little value in this message: it's not easy to say which particular
> guest triggered it so it's unclear what system administrator is supposed to do
> upon seeing this message.
> 
> Also, while on it, I think kvm_lapic_enable_pv_eoi() is misnamed: it is also used
> for *disabling* PV EOI.
> 
> Instead of dropping KVM_MSR_ENABLED bit, I'd suggest we only set
> vcpu->arch.pv_eoi.msr_val in case of success. In case
> kvm_gfn_to_hva_cache_init() fails, we inject #GP so it's reasonable to expect
> that MSR's value didn't change.
> 


Hi Vitaly:

Could you submit your patch? 

Thanks

-Li