linux-kernel - Re: [PATCH 1/2] KVM: x86: avoid simultaneous queueing of both IRQ and SMI

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: <20160601164024.GF30721@potion>
Date:	Wed, 1 Jun 2016 18:40:24 +0200
From:	Radim Krčmář <rkrcmar@...hat.com>
To:	Paolo Bonzini <pbonzini@...hat.com>
Cc:	linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
	stable@...r.kernel.org
Subject: Re: [PATCH 1/2] KVM: x86: avoid simultaneous queueing of both IRQ
 and SMI

2016-06-01 14:35+0200, Paolo Bonzini:
> If the processor exits to KVM while delivering an interrupt,
> the hypervisor then requeues the interrupt for the next vmentry.
> Trying to enter SMM in this same window causes to enter non-root
> mode in emulated SMM (i.e. with IF=0) and with a request to
> inject an IRQ (i.e. with a valid VM-entry interrupt info field).
> This is invalid guest state (SDM 26.3.1.4 "Check on Guest RIP
> and RFLAGS") and the processor fails vmentry.
> 
> The fix is to defer the injection from KVM_REQ_SMI to KVM_REQ_EVENT,
> like we already do for e.g. NMIs.  This patch doesn't change the
> name of the process_smi function so that it can be applied to
> stable releases.  The next patch will modify the names so that
> process_nmi and process_smi process respectively KVM_REQ_NMI and
> KVM_REQ_SMI.
> 
> This is especially common with Windows, probably due to the
> self-IPI trick that it uses to deliver deferred procedure
> calls (DPCs).
> 
> Reported-by: Laszlo Ersek <lersek@...hat.com>
> Fixes: 64d6067057d9658acb8675afcfba549abdb7fc16
> Cc: stable@...r.kernel.org
> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> ---
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> @@ -6098,7 +6094,10 @@ static int inject_pending_event(struct kvm_vcpu *vcpu, bool req_int_win)
>  	}
>  
>  	/* try to inject new event if pending */
> -	if (vcpu->arch.nmi_pending && kvm_x86_ops->nmi_allowed(vcpu)) {
> +	if (vcpu->arch.smi_pending && !is_smm(vcpu)) {

Clearing smi_pending in kvm_vcpu_reset() would be safer now that SMI can
be injected without a request or RSM.

> +		--vcpu->arch.smi_pending;

(I'd use 'vcpu->arch.smi_pending = false', to make it clearer that we
 don't want multiple pending SMIs, unlike NMIs.  smi_pending is bool,
 so the generated code should be identical.)

> +		process_smi(vcpu);
> +	} else if (vcpu->arch.nmi_pending && kvm_x86_ops->nmi_allowed(vcpu)) {
>  		--vcpu->arch.nmi_pending;


> @@ -6621,8 +6631,10 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>  
>  	kvm_load_guest_xcr0(vcpu);
>  
> -	if (req_immediate_exit)
> +	if (req_immediate_exit) {
> +		kvm_make_request(KVM_REQ_EVENT, vcpu);
>  		smp_send_reschedule(vcpu->cpu);

(Is this a fix for non-smi cases too?)

Thanks.