linux-kernel - Re: [PATCH v3 2/4] KVM: x86: Use vector-hashing to deliver lowest-priority interrupts

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: <20160121172109.GB17514@potion.brq.redhat.com>
Date:	Thu, 21 Jan 2016 18:21:10 +0100
From:	"rkrcmar@...hat.com" <rkrcmar@...hat.com>
To:	"Wu, Feng" <feng.wu@...el.com>
Cc:	Yang Zhang <yang.zhang.wz@...il.com>,
	"pbonzini@...hat.com" <pbonzini@...hat.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>
Subject: Re: [PATCH v3 2/4] KVM: x86: Use vector-hashing to deliver
 lowest-priority interrupts

2016-01-21 05:33+0000, Wu, Feng:
>> From: linux-kernel-owner@...r.kernel.org [mailto:linux-kernel-
>> owner@...r.kernel.org] On Behalf Of Yang Zhang
>> On 2016/1/20 9:42, Feng Wu wrote:
>> > +			/*
>> > +			 * We may find a hardware disabled LAPIC here, if
>> that
>> > +			 * is the case, print out a error message once for each
>> > +			 * guest and return.
>> > +			 */
>> > +			if (!dst[idx-1] &&
>> > +				(kvm->arch.disabled_lapic_found == 0)) {
>> > +				kvm->arch.disabled_lapic_found = 1;
>> > +				printk(KERN_ERR
>> > +					"Disabled LAPIC found during irq
>> injection\n");
>> > +				goto out;
>> 
>> What does "goto out" mean? Inject successfully or fail? According the
>> value of ret which is set to ture here, it means inject successfully but

(true actually means that fast path did the job and slow path isn't
 needed.)

>> i = -1.

(I think there isn't a practical difference between *r=-1 and *r=0.)

> Oh, I didn't notice 'ret' is initialized to true, I thought it was initialized
> to false like another function, I should add a "ret = false' here. We should
> failed to inject the interrupt since hardware disabled LAPIC is found.

'ret = true' is the better one.  We know that the interrupt is not
deliverable [1], so there's no point in trying to deliver with the slow
path.  We behave similarly when the interrupt targets a single disabled
APIC.

---
1: Well ... it's possible that slowpath would deliver it thanks to
   different handling of disabled APICs, but it's undefined behavior,
   so it doesn't matter matter if we don't try.