linux-kernel - Re: [PATCH] KVM: nVMX: Emulate NOPs in L2, and PAUSE if it's not intercepted

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: <ZDWFup/igRUtfNTa@google.com>
Date:   Tue, 11 Apr 2023 09:07:22 -0700
From:   Sean Christopherson <seanjc@...gle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Mathias Krause <minipli@...ecurity.net>
Subject: Re: [PATCH] KVM: nVMX: Emulate NOPs in L2, and PAUSE if it's not intercepted

On Tue, Apr 11, 2023, Paolo Bonzini wrote:
> On 4/5/23 02:23, Sean Christopherson wrote:
> > Fixes: 07721feee46b ("KVM: nVMX: Don't emulate instructions in guest mode")
> > Cc: Mathias Krause <minipli@...ecurity.net>
> > Cc: stable@...r.kernel.org
> > Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> > ---
> >   arch/x86/kvm/vmx/vmx.c | 15 +++++++++++++++
> >   1 file changed, 15 insertions(+)
> > 
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index 9ae4044f076f..1e560457bf9a 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -7898,6 +7898,21 @@ static int vmx_check_intercept(struct kvm_vcpu *vcpu,
> >   		/* FIXME: produce nested vmexit and return X86EMUL_INTERCEPTED.  */
> >   		break;
> > +	case x86_intercept_pause:
> > +		/*
> > +		 * PAUSE is a single-byte NOP with a REPE prefix, i.e. collides
> > +		 * with vanilla NOPs in the emulator.  Apply the interception
> > +		 * check only to actual PAUSE instructions.  Don't check
> > +		 * PAUSE-loop-exiting, software can't expect a given PAUSE to
> > +		 * exit, i.e. KVM is within its rights to allow L2 to execute
> > +		 * the PAUSE.
> > +		 */
> > +		if ((info->rep_prefix != REPE_PREFIX) ||
> > +		    !nested_cpu_has2(vmcs12, CPU_BASED_PAUSE_EXITING))
> > +			return X86EMUL_CONTINUE;
> > +
> > +		break;
> > +
> >   	/* TODO: check more intercepts... */
> >   	default:
> >   		break;
> > 
> > base-commit: 27d6845d258b67f4eb3debe062b7dacc67e0c393
> 
> Reviewed-by: Paolo Bonzini <pbonzini@...hat.com>
> 
> Would you like me to apply this for 6.3?

Nah, I don't think there's a good risk vs. reward ratio.  KVM doesn't enable PLE
for L2, never enables PAUSE-exiting, and won't emulate NOP without forced emulation
or TLB shenanigans from L2.  So other than tests, this really shouldn't matter.

Actually, typing that out is making me rethink the stable tag...