linux-kernel - Re: [PATCH] kvm: x86: fix a race condition result to lost INIT

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>] [day] [month] [year] [list]

Message-ID: <CANRm+CzWiZttYzRovtYSXgMJ3DaHt9jqwN8N-m7441u29KD+Hg@mail.gmail.com>
Date:   Sun, 30 Jul 2017 19:53:53 +0800
From:   Wanpeng Li <kernellwp@...il.com>
To:     Peng Hao <peng.hao2@....com.cn>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krcmar <rkrcmar@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        kvm <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] kvm: x86: fix a race condition result to lost INIT

2017-07-30 19:33 GMT+08:00  <peng.hao2@....com.cn>:
>
>
>
>
>>2017-07-30 18:42 GMT+08:00  <peng.hao2@....com.cn>:
>
>>>> 2017-07-31 0:24 GMT+08:00 Peng Hao <peng.hao2@....com.cn>:
>>>
>>>> > when SMP VM start, AP may lost INIT because of receiving INIT between
>>>> > kvm_vcpu_ioctl_x86_get/set_vcpu_events.
>>>> >
>>>> >    vcpu 0                             vcpu 1
>>>> >                                kvm_vcpu_ioctl_x86_get_vcpu_events
>>>> >                                        events->smi.latched_init=0
>>>> >  send INIT to vcpu1
>>>> >    set vcpu1's pending_events
>>>> >                                kvm_vcpu_ioctl_x86_set_vcpu_events
>>>> >                                     events->smi.latched_init == 0
>>>> >                                       clear INIT in pending_events
>>>> > I don't think it need set/clear kernel state according to userspace's
>>>> > info.
>>>> >
>>>> > Signed-off-by: Peng Hao <peng.hao2@....com.cn>
>>>> > ---
>>>> >  arch/x86/kvm/x86.c | 6 ------
>>>> >  1 file changed, 6 deletions(-)
>>>> >
>>>> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>>> > index 6c7266f..393a7b7 100644
>>>> > --- a/arch/x86/kvm/x86.c
>>>> > +++ b/arch/x86/kvm/x86.c
>>>> > @@ -3157,12 +3157,6 @@ static int
>>>> > kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
>>>> >                         vcpu->arch.hflags |= HF_SMM_INSIDE_NMI_MASK;
>>>> >                 else
>>>> >                         vcpu->arch.hflags &= ~HF_SMM_INSIDE_NMI_MASK;
>>>> > -               if (lapic_in_kernel(vcpu)) {
>>>> > -                       if (events->smi.latched_init)
>>>> > -                               set_bit(KVM_APIC_INIT,
>>>> > &vcpu->arch.apic->pending_events);
>>>
>>>> This is not correct, you will lose the INIT after live migration. I
>>>> just send out another patch to fix it. Thanks for the report.
>>>
>>> yes, you're right. I should modify on qemu not kernel.
>
>>There is a patch here to fix it in kvm, https://lkml.org/lkml/2017/7/30/72
>
>>>
>
> I was affected by sipi, kernel never report  valid sipi to user space,

How you afftected by SIPI? In addition, you reported the lost of INIT.
Is it observed by your workload or by code review?

Regards,
Wanpeng Li

>
> we don't care whether sipi is lost  after migration. Why?
>
>>>> > -                       else
>>>> > -                               clear_bit(KVM_APIC_INIT,
>>>> > &vcpu->arch.apic->pending_events);
>>>> > -               }
>>>> >         }
>
>
>