re: KVM: x86: Add lowest-priority support for vt-d posted-interrupts

re: KVM: x86: Add lowest-priority support for vt-d posted-interrupts

[Dan Carpenter]

Hello Feng Wu,

This is a semi-automatic email about new static checker warnings.

The patch 6228a0da8057: "KVM: x86: Add lowest-priority support for 
vt-d posted-interrupts" from Jan 25, 2016, leads to the following 
Smatch complaint:

arch/x86/kvm/lapic.c:875 kvm_intr_is_single_vcpu_fast()
	 error: we previously assumed 'dst' could be null (see line 868)

arch/x86/kvm/lapic.c
   867				dst = map->logical_map[cid][idx];
   868				if (!dst && !kvm->arch.disabled_lapic_found) {
                                    ^^^^
Patch introduces a new assignment and NULL check.

   869					kvm->arch.disabled_lapic_found = true;
   870					printk(KERN_INFO
   871						"Disabled LAPIC found during irq injection\n");
   872					goto out;
   873				}
   874	
   875				*dest_vcpu = dst->vcpu;
                                             ^^^^^^^^^
Old unchecked dereference.

   876			} else {
   877				for_each_set_bit(i, &bitmap, 16) {

regards,
dan carpenter

[PATCH] KVM: x86: fix *NULL on invalid low-prio irq

[Radim Krčmář]

Smatch noticed a NULL dereference in kvm_intr_is_single_vcpu_fast that
happens if VM already warned about invalid lowest-priority interrupt.

Create a function for common code while fixing it.

Fixes: 6228a0da8057 ("KVM: x86: Add lowest-priority support for vt-d posted-interrupts")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 arch/x86/kvm/lapic.c | 31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 1482a581a83c..cf74404230ca 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -685,6 +685,15 @@ int kvm_vector_to_index(u32 vector, u32 dest_vcpus,
 	return idx;
 }
 
+static void kvm_apic_disabled_lapic_found(struct kvm *kvm)
+{
+	if (!kvm->arch.disabled_lapic_found) {
+		kvm->arch.disabled_lapic_found = true;
+		printk(KERN_INFO
+		       "Disabled LAPIC found during irq injection\n");
+	}
+}
+
 bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
 		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
 {
@@ -763,15 +772,8 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
 			idx = kvm_vector_to_index(irq->vector,
 				dest_vcpus, &bitmap, 16);
 
-			/*
-			 * 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] && !kvm->arch.disabled_lapic_found) {
-				kvm->arch.disabled_lapic_found = true;
-				printk(KERN_INFO
-					"Disabled LAPIC found during irq injection\n");
+			if (!dst[idx]) {
+				kvm_apic_disabled_lapic_found(kvm);
 				goto out;
 			}
 
@@ -859,16 +861,9 @@ bool kvm_intr_is_single_vcpu_fast(struct kvm *kvm, struct kvm_lapic_irq *irq,
 			idx = kvm_vector_to_index(irq->vector, dest_vcpus,
 						  &bitmap, 16);
 
-			/*
-			 * We may find a hardware disabled LAPIC here, if that
-			 * is the case, print out a error message once for each
-			 * guest and return
-			 */
 			dst = map->logical_map[cid][idx];
-			if (!dst && !kvm->arch.disabled_lapic_found) {
-				kvm->arch.disabled_lapic_found = true;
-				printk(KERN_INFO
-					"Disabled LAPIC found during irq injection\n");
+			if (!dst) {
+				kvm_apic_disabled_lapic_found(kvm);
 				goto out;
 			}
 
-- 
2.7.1

RE: [PATCH] KVM: x86: fix *NULL on invalid low-prio irq

[Wu, Feng]

> -----Original Message-----
> From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> Sent: Friday, February 12, 2016 10:00 PM
> To: linux@vger.kernel.org
> Cc: kvm@vger.kernel.org; Dan Carpenter <dan.carpenter@oracle.com>; Wu,
> Feng <feng.wu@intel.com>; Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH] KVM: x86: fix *NULL on invalid low-prio irq
> 
> Smatch noticed a NULL dereference in kvm_intr_is_single_vcpu_fast that
> happens if VM already warned about invalid lowest-priority interrupt.
> 
> Create a function for common code while fixing it.

Thanks for the fix, Radim! I just saw the report by Dan and cooked a patch
for it. Your patch looks good :)

Thanks,
Feng

Re: [PATCH] KVM: x86: fix *NULL on invalid low-prio irq

[Radim Krcmár]

2016-02-12 14:10+0000, Wu, Feng:
>> From: Radim Krčmář [mailto:rkrcmar@redhat.com]
>> Smatch noticed a NULL dereference in kvm_intr_is_single_vcpu_fast that
>> happens if VM already warned about invalid lowest-priority interrupt.
>> 
>> Create a function for common code while fixing it.
> 
> Thanks for the fix, Radim! I just saw the report by Dan and cooked a patch
> for it.

(Sorry for the duplication of effort, I will wait longer next time.)

>         Your patch looks good :)

Thanks.

RE: [PATCH] KVM: x86: fix *NULL on invalid low-prio irq

[Wu, Feng]

> -----Original Message-----
> From: Radim Krcmár [mailto:rkrcmar@redhat.com]
> Sent: Friday, February 12, 2016 10:28 PM
> To: Wu, Feng <feng.wu@intel.com>
> Cc: linux@vger.kernel.org; kvm@vger.kernel.org; Dan Carpenter
> <dan.carpenter@oracle.com>; Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH] KVM: x86: fix *NULL on invalid low-prio irq
> 
> 2016-02-12 14:10+0000, Wu, Feng:
> >> From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> >> Smatch noticed a NULL dereference in kvm_intr_is_single_vcpu_fast that
> >> happens if VM already warned about invalid lowest-priority interrupt.
> >>
> >> Create a function for common code while fixing it.
> >
> > Thanks for the fix, Radim! I just saw the report by Dan and cooked a patch
> > for it.
> 
> (Sorry for the duplication of effort, I will wait longer next time.)

No problem, that's fine. I didn't notice the report in time due to holiday.
But it is good to having you help to fix it! :)

Thanks,
Feng

> 
> >         Your patch looks good :)
> 
> Thanks.