Re: [PATCH] KVM: LAPIC: Fix pv ipis out-of-bounds access

[Radim Krcmar]

2018-08-29 13:43+0300, Liran Alon:
> Why is “min” defined as “int” instead of “unsigned int”?
> It represents the lowest APIC ID in bitmap so it can’t be negative…

Right,

I think the code would look better as something like (untested):

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 0cefba28c864..24fc84eb97d2 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -548,7 +548,7 @@ int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct 
kvm_lapic_irq *irq,
 }
 
 int kvm_pv_send_ipi(struct kvm *kvm, unsigned long ipi_bitmap_low,
-                   unsigned long ipi_bitmap_high, int min,
+                   unsigned long ipi_bitmap_high, u32 min,
                    unsigned long icr, int op_64_bit)
 {
        int i;
@@ -557,6 +557,7 @@ int kvm_pv_send_ipi(struct kvm *kvm, unsigned long 
ipi_bitmap_low,
        struct kvm_lapic_irq irq = {0};
        int cluster_size = op_64_bit ? 64 : 32;
        int count = 0;
+       unsigned long ipi_bitmap[2] = {ipi_bitmap_low, ipi_bitmap_high};
 
        irq.vector = icr & APIC_VECTOR_MASK;
        irq.delivery_mode = icr & APIC_MODE_MASK;
@@ -571,16 +572,14 @@ int kvm_pv_send_ipi(struct kvm *kvm, unsigned long 
ipi_bitmap_low,
        rcu_read_lock();
        map = rcu_dereference(kvm->arch.apic_map);
 
-       /* Bits above cluster_size are masked in the caller.  */
-       for_each_set_bit(i, &ipi_bitmap_low, BITS_PER_LONG) {
-               vcpu = map->phys_map[min + i]->vcpu;
-               count += kvm_apic_set_irq(vcpu, &irq, NULL);
-       }
+       if (min <= map->max_apic_id) {
+               size_t ipi_bitmap_size = MIN(sizeof(ipi_bitmap) * 8,
+                                            map->max_apic_id - min + 1);
 
-       min += cluster_size;
-       for_each_set_bit(i, &ipi_bitmap_high, BITS_PER_LONG) {
-               vcpu = map->phys_map[min + i]->vcpu;
-               count += kvm_apic_set_irq(vcpu, &irq, NULL);
+               for_each_set_bit(i, ipi_bitmap, ipi_bitmap_size) {
+                       vcpu = map->phys_map[min + i]->vcpu;
+                       count += kvm_apic_set_irq(vcpu, &irq, NULL);
+               }
        }
 
        rcu_read_unlock();