"Li,Rongqing" <lirongqing@xxxxxxxxx> writes:
>> -----邮件原件-----
>> 发件人: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
>> 发送时间: 2021年11月5日 17:08
>> 收件人: Li,Rongqing <lirongqing@xxxxxxxxx>
>> 抄送: kvm@xxxxxxxxxxxxxxx; pbonzini@xxxxxxxxxx; seanjc@xxxxxxxxxx;
>> Li,Rongqing <lirongqing@xxxxxxxxx>
>> 主题: Re: [PATCH] KVM: x86: disable pv eoi if guest gives a wrong address
>>
>> Li RongQing <lirongqing@xxxxxxxxx> 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@xxxxxxxxx>
>> > ---
>> > 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?
>
Sure, I will.
--
Vitaly
