Skip to contentSkip to conversation

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

PATCH In review Dan Carpenter 3 participants 5 messages Feb 11, 2016 – Feb 12, 2016

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

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

#1 lore raw

PATCH 1/1 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.

Fixes 6228a0da8057 ("KVM: x86: Add lowest-priority support for vt-d posted-interrupts")
---
 arch/x86/kvm/lapic.c | 31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)
arch/x86/kvm/lapic.c +13 -18
index 1482a581a83c..cf74404230ca 100644
@@ -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;
 			}
 
Signature · 1 line
2.7.1
#2 lore raw

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

Quoted 10 lines

-----Original Message-----
From: Radim Krčmář [mailto:[email protected]]
Sent: Friday, February 12, 2016 10:00 PM
To: [email protected]
Cc: [email protected]; Dan Carpenter <[email protected]>; Wu, Feng <[email protected]>; Paolo Bonzini <[email protected]>
Subject: [PATCH] KVM: x86: fix *NULL on invalid low-prio irq

Quoted 4 lines

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

#3 lore raw

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

2016-02-12 14:10+0000, Wu, Feng:

Quoted 8 lines
Quoted 5 lines

From: Radim Krčmář [mailto:[email protected]]
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.)

Quoted 1 line

Your patch looks good :)

Thanks.

#4 lore raw

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

Quoted 15 lines

-----Original Message-----
From: Radim Krcmár [mailto:[email protected]]
Sent: Friday, February 12, 2016 10:28 PM
To: Wu, Feng <[email protected]>
Cc: [email protected]; [email protected]; Dan Carpenter <[email protected]>; Paolo Bonzini <[email protected]>
Subject: Re: [PATCH] KVM: x86: fix *NULL on invalid low-prio irq

2016-02-12 14:10+0000, Wu, Feng:

Quoted 4 lines
Quoted 4 lines

From: Radim Krčmář [mailto:[email protected]]
Smatch noticed a NULL dereference in kvm_intr_is_single_vcpu_fast that happens if VM already warned about invalid lowest-priority interrupt.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

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

Quoted 4 lines
Quoted 2 lines

Your patch looks good :)

Thanks.