linux-kernel - Re: [syzbot] WARNING in nested_vmx

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

Message-ID: <Ya5G5eQrGNRIwbcm@google.com>
Date:   Mon, 6 Dec 2021 17:22:45 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Vitaly Kuznetsov <vkuznets@...hat.com>
Cc:     Maxim Levitsky <mlevitsk@...hat.com>, kvm@...r.kernel.org,
        jmattson@...gle.com,
        syzbot <syzbot+f1d2136db9c80d4733e8@...kaller.appspotmail.com>,
        bp@...en8.de, dave.hansen@...ux.intel.com, hpa@...or.com,
        joro@...tes.org, linux-kernel@...r.kernel.org, mingo@...hat.com,
        pbonzini@...hat.com, syzkaller-bugs@...glegroups.com,
        tglx@...utronix.de, wanpengli@...cent.com, x86@...nel.org
Subject: Re: [syzbot] WARNING in nested_vmx_vmexit

On Mon, Dec 06, 2021, Sean Christopherson wrote:
> On Mon, Dec 06, 2021, Vitaly Kuznetsov wrote:
> > Sean Christopherson <seanjc@...gle.com> writes:
> > > I objected to the patch[*], but looking back at the dates, it appears that I did
> > > so after the patch was queued and my comments were never addressed.  
> > > I'll see if I can reproduce this with a selftest.  The fix is likely just:
> > >
> > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > > index dc4909b67c5c..927a7c43b73b 100644
> > > --- a/arch/x86/kvm/vmx/vmx.c
> > > +++ b/arch/x86/kvm/vmx/vmx.c
> > > @@ -6665,10 +6665,6 @@ static fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu)
> > >          * consistency check VM-Exit due to invalid guest state and bail.
> > >          */
> > >         if (unlikely(vmx->emulation_required)) {
> > > -
> > > -               /* We don't emulate invalid state of a nested guest */
> > > -               vmx->fail = is_guest_mode(vcpu);
> > > -
> > >                 vmx->exit_reason.full = EXIT_REASON_INVALID_STATE;
> > >                 vmx->exit_reason.failed_vmentry = 1;
> > >                 kvm_register_mark_available(vcpu, VCPU_EXREG_EXIT_INFO_1);
> > >
> > > [*] https://lore.kernel.org/all/YWDWPbgJik5spT1D@google.com/
> > 
> > Let's also summon Max to the discussion to get his thoughts.
> 
> Thinking more on this, we should do three things:

Doh, hit send too soon.  s/three/five, I'm not _that_ bad at math...

>   1. Revert this part back to "vmx->fail = 0".
> 
>   2. Override SS.RPL and CS.RPL on RSM for VMX.  Not sure this is strictly
>      necessary, I'm struggling to remember how SS.RPL and SS.DPL can get out of
>      sync.
> 
>        IF RFLAGS.VM = 0 AND (in VMX root operation OR the “unrestricted guest”
>        VM-execution control is 0)
>        THEN
>          CS.RPL := SS.DPL;
>          SS.RPL := SS.DPL;
>        FI;
> 
>   3. Modify RSM to go into TRIPLE_FAULT if vmx->emulation_required is true after
>      loading state for RSM.  On AMD, whose SMRAM KVM emulates, all segment state
>      is read-only, i.e. if it's modified to be invalid then KVM essentially do
>      whatever it wants.
> 
>   4. Reject KVM_RUN if is_guest_mode() and vmx->emulation_required are true.  By
>      handling the RSM case explicitly, this means userspace has attempted to run
>      L2 with garbage, which KVM most definitely doesn't want to support.
> 
>   5. Add KVM_BUG_ON(is_guest_mode(vcpu), ...) in the emulation_required path in
>      vmx_vcpu_run(), as reaching that point means KVM botched VM-Enter, RSM or
>      the #4 above.