Hello Wanpeng Li,
The patch 4180bf1b655a: "KVM: X86: Implement "send IPI" hypercall"
from Jul 23, 2018, leads to the following static checker warning:
arch/x86/kvm/lapic.c:576 kvm_pv_send_ipi()
error: buffer underflow 'map->phys_map' 's32min-s32max'
arch/x86/kvm/lapic.c
550 int kvm_pv_send_ipi(struct kvm *kvm, unsigned long ipi_bitmap_low,
551 unsigned long ipi_bitmap_high, int min,
552 unsigned long icr, int op_64_bit)
553 {
554 int i;
555 struct kvm_apic_map *map;
556 struct kvm_vcpu *vcpu;
557 struct kvm_lapic_irq irq = {0};
558 int cluster_size = op_64_bit ? 64 : 32;
559 int count = 0;
560
561 irq.vector = icr & APIC_VECTOR_MASK;
562 irq.delivery_mode = icr & APIC_MODE_MASK;
563 irq.level = (icr & APIC_INT_ASSERT) != 0;
564 irq.trig_mode = icr & APIC_INT_LEVELTRIG;
565
566 if (icr & APIC_DEST_MASK)
567 return -KVM_EINVAL;
568 if (icr & APIC_SHORT_MASK)
569 return -KVM_EINVAL;
570
571 rcu_read_lock();
572 map = rcu_dereference(kvm->arch.apic_map);
573
574 /* Bits above cluster_size are masked in the caller. */
575 for_each_set_bit(i, &ipi_bitmap_low, BITS_PER_LONG) {
576 vcpu = map->phys_map[min + i]->vcpu;
^^^^^^^
577 count += kvm_apic_set_irq(vcpu, &irq, NULL);
578 }
579
580 min += cluster_size;
581 for_each_set_bit(i, &ipi_bitmap_high, BITS_PER_LONG) {
582 vcpu = map->phys_map[min + i]->vcpu;
^^^^^^^
The truth is that I don't understand KVM very well so I'm just reporting
these warning in the hope that someone will explain things to me. This
is called from kvm_emulate_hypercall(). Recently we marked the return
from kvm_register_read() as untrusted data. Now Smatch thinks that
"min" is totally untrusted and there are no boundary checks.
Btw the reason Smatch only warns about a buffer underflow instead of a
buffer overflow is because it doesn't know the size of map->phys_map[].
When can kvm_register_read() be trusted?
583 count += kvm_apic_set_irq(vcpu, &irq, NULL);
584 }
585
586 rcu_read_unlock();
587 return count;
588 }
regards,
dan carpenter
