[PATCH] KVM: x86: fix out-of-bounds accesses of rtc_eoi map

[PATCH] KVM: x86: fix out-of-bounds accesses of rtc_eoi map

[Radim Krčmář]

KVM was using arrays of size KVM_MAX_VCPUS with vcpu_id, but ID can be
bigger that the maximal number of VCPUs, resulting in out-of-bounds
access.

Found by syzkaller:

  BUG: KASAN: slab-out-of-bounds in __apic_accept_irq+0xb33/0xb50 at addr [...]
  Write of size 1 by task a.out/27101
  CPU: 1 PID: 27101 Comm: a.out Not tainted 4.9.0-rc5+ #49
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
   [...]
  Call Trace:
   [...] __apic_accept_irq+0xb33/0xb50 arch/x86/kvm/lapic.c:905
   [...] kvm_apic_set_irq+0x10e/0x180 arch/x86/kvm/lapic.c:495
   [...] kvm_irq_delivery_to_apic+0x732/0xc10 arch/x86/kvm/irq_comm.c:86
   [...] ioapic_service+0x41d/0x760 arch/x86/kvm/ioapic.c:360
   [...] ioapic_set_irq+0x275/0x6c0 arch/x86/kvm/ioapic.c:222
   [...] kvm_ioapic_inject_all arch/x86/kvm/ioapic.c:235
   [...] kvm_set_ioapic+0x223/0x310 arch/x86/kvm/ioapic.c:670
   [...] kvm_vm_ioctl_set_irqchip arch/x86/kvm/x86.c:3668
   [...] kvm_arch_vm_ioctl+0x1a08/0x23c0 arch/x86/kvm/x86.c:3999
   [...] kvm_vm_ioctl+0x1fa/0x1a70 arch/x86/kvm/../../../virt/kvm/kvm_main.c:3099

Reported-by: Dmitry Vyukov <dvyukov@google.com>
Cc: stable@vger.kernel.org
Fixes: af1bae5497b9 ("KVM: x86: bump KVM_MAX_VCPU_ID to 1023")
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Steve Rutherford <srutherford@google.com>
---
 arch/x86/kvm/ioapic.c | 2 +-
 arch/x86/kvm/ioapic.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 1a22de70f7f7..6e219e5c07d2 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -94,7 +94,7 @@ static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic,
 static void rtc_irq_eoi_tracking_reset(struct kvm_ioapic *ioapic)
 {
 	ioapic->rtc_status.pending_eoi = 0;
-	bitmap_zero(ioapic->rtc_status.dest_map.map, KVM_MAX_VCPUS);
+	bitmap_zero(ioapic->rtc_status.dest_map.map, KVM_MAX_VCPU_ID);
 }
 
 static void kvm_rtc_eoi_tracking_restore_all(struct kvm_ioapic *ioapic);
diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
index 7d2692a49657..1cc6e54436db 100644
--- a/arch/x86/kvm/ioapic.h
+++ b/arch/x86/kvm/ioapic.h
@@ -42,13 +42,13 @@ struct kvm_vcpu;
 
 struct dest_map {
 	/* vcpu bitmap where IRQ has been sent */
-	DECLARE_BITMAP(map, KVM_MAX_VCPUS);
+	DECLARE_BITMAP(map, KVM_MAX_VCPU_ID);
 
 	/*
 	 * Vector sent to a given vcpu, only valid when
 	 * the vcpu's bit in map is set
 	 */
-	u8 vectors[KVM_MAX_VCPUS];
+	u8 vectors[KVM_MAX_VCPU_ID];
 };
 
 
-- 
2.10.2

Re: [PATCH] KVM: x86: fix out-of-bounds accesses of rtc_eoi map

[Paolo Bonzini]

On 23/11/2016 21:15, Radim Krčmář wrote:
> KVM was using arrays of size KVM_MAX_VCPUS with vcpu_id, but ID can be
> bigger that the maximal number of VCPUs, resulting in out-of-bounds
> access.
> 
> Found by syzkaller:
> 
>   BUG: KASAN: slab-out-of-bounds in __apic_accept_irq+0xb33/0xb50 at addr [...]
>   Write of size 1 by task a.out/27101
>   CPU: 1 PID: 27101 Comm: a.out Not tainted 4.9.0-rc5+ #49
>   Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>    [...]
>   Call Trace:
>    [...] __apic_accept_irq+0xb33/0xb50 arch/x86/kvm/lapic.c:905
>    [...] kvm_apic_set_irq+0x10e/0x180 arch/x86/kvm/lapic.c:495
>    [...] kvm_irq_delivery_to_apic+0x732/0xc10 arch/x86/kvm/irq_comm.c:86
>    [...] ioapic_service+0x41d/0x760 arch/x86/kvm/ioapic.c:360
>    [...] ioapic_set_irq+0x275/0x6c0 arch/x86/kvm/ioapic.c:222
>    [...] kvm_ioapic_inject_all arch/x86/kvm/ioapic.c:235
>    [...] kvm_set_ioapic+0x223/0x310 arch/x86/kvm/ioapic.c:670
>    [...] kvm_vm_ioctl_set_irqchip arch/x86/kvm/x86.c:3668
>    [...] kvm_arch_vm_ioctl+0x1a08/0x23c0 arch/x86/kvm/x86.c:3999
>    [...] kvm_vm_ioctl+0x1fa/0x1a70 arch/x86/kvm/../../../virt/kvm/kvm_main.c:3099
> 
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Cc: stable@vger.kernel.org
> Fixes: af1bae5497b9 ("KVM: x86: bump KVM_MAX_VCPU_ID to 1023")
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Steve Rutherford <srutherford@google.com>
> ---
>  arch/x86/kvm/ioapic.c | 2 +-
>  arch/x86/kvm/ioapic.h | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
> index 1a22de70f7f7..6e219e5c07d2 100644
> --- a/arch/x86/kvm/ioapic.c
> +++ b/arch/x86/kvm/ioapic.c
> @@ -94,7 +94,7 @@ static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic,
>  static void rtc_irq_eoi_tracking_reset(struct kvm_ioapic *ioapic)
>  {
>  	ioapic->rtc_status.pending_eoi = 0;
> -	bitmap_zero(ioapic->rtc_status.dest_map.map, KVM_MAX_VCPUS);
> +	bitmap_zero(ioapic->rtc_status.dest_map.map, KVM_MAX_VCPU_ID);
>  }
>  
>  static void kvm_rtc_eoi_tracking_restore_all(struct kvm_ioapic *ioapic);
> diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
> index 7d2692a49657..1cc6e54436db 100644
> --- a/arch/x86/kvm/ioapic.h
> +++ b/arch/x86/kvm/ioapic.h
> @@ -42,13 +42,13 @@ struct kvm_vcpu;
>  
>  struct dest_map {
>  	/* vcpu bitmap where IRQ has been sent */
> -	DECLARE_BITMAP(map, KVM_MAX_VCPUS);
> +	DECLARE_BITMAP(map, KVM_MAX_VCPU_ID);
>  
>  	/*
>  	 * Vector sent to a given vcpu, only valid when
>  	 * the vcpu's bit in map is set
>  	 */
> -	u8 vectors[KVM_MAX_VCPUS];
> +	u8 vectors[KVM_MAX_VCPU_ID];
>  };
>  
>  
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Prasad, this needs a CVE.

Paolo

Re: [PATCH] KVM: x86: fix out-of-bounds accesses of rtc_eoi map

[P J P]

[-- Attachment #1: Type: text/plain, Size: 1479 bytes --]

+-- On Wed, 23 Nov 2016, Paolo Bonzini wrote --+
| On 23/11/2016 21:15, Radim Krčmář wrote:
| > KVM was using arrays of size KVM_MAX_VCPUS with vcpu_id, but ID can be
| > bigger that the maximal number of VCPUs, resulting in out-of-bounds
| > access.
| > 
| > Found by syzkaller:
| > 
| >   BUG: KASAN: slab-out-of-bounds in __apic_accept_irq+0xb33/0xb50 at addr [...]
| >   Write of size 1 by task a.out/27101
| >   CPU: 1 PID: 27101 Comm: a.out Not tainted 4.9.0-rc5+ #49
| >   Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
| >    [...]
| >   Call Trace:
| >    [...] __apic_accept_irq+0xb33/0xb50 arch/x86/kvm/lapic.c:905
| >    [...] kvm_apic_set_irq+0x10e/0x180 arch/x86/kvm/lapic.c:495
| >    [...] kvm_irq_delivery_to_apic+0x732/0xc10 arch/x86/kvm/irq_comm.c:86
| >    [...] ioapic_service+0x41d/0x760 arch/x86/kvm/ioapic.c:360
| >    [...] ioapic_set_irq+0x275/0x6c0 arch/x86/kvm/ioapic.c:222
| >    [...] kvm_ioapic_inject_all arch/x86/kvm/ioapic.c:235
| >    [...] kvm_set_ioapic+0x223/0x310 arch/x86/kvm/ioapic.c:670
| >    [...] kvm_vm_ioctl_set_irqchip arch/x86/kvm/x86.c:3668
| >    [...] kvm_arch_vm_ioctl+0x1a08/0x23c0 arch/x86/kvm/x86.c:3999
| >    [...] kvm_vm_ioctl+0x1fa/0x1a70 arch/x86/kvm/../../../virt/kvm/kvm_main.c:3099
| 
| Prasad, this needs a CVE.

  Yes, I'm processing it. Thank you so much for CC'ing.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [PATCH] KVM: x86: fix out-of-bounds accesses of rtc_eoi map

[David Hildenbrand]

Am 23.11.2016 um 21:15 schrieb Radim Krčmář:
> KVM was using arrays of size KVM_MAX_VCPUS with vcpu_id, but ID can be
> bigger that the maximal number of VCPUs, resulting in out-of-bounds
> access.
>
> Found by syzkaller:
>
>   BUG: KASAN: slab-out-of-bounds in __apic_accept_irq+0xb33/0xb50 at addr [...]
>   Write of size 1 by task a.out/27101
>   CPU: 1 PID: 27101 Comm: a.out Not tainted 4.9.0-rc5+ #49
>   Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>    [...]
>   Call Trace:
>    [...] __apic_accept_irq+0xb33/0xb50 arch/x86/kvm/lapic.c:905
>    [...] kvm_apic_set_irq+0x10e/0x180 arch/x86/kvm/lapic.c:495
>    [...] kvm_irq_delivery_to_apic+0x732/0xc10 arch/x86/kvm/irq_comm.c:86
>    [...] ioapic_service+0x41d/0x760 arch/x86/kvm/ioapic.c:360
>    [...] ioapic_set_irq+0x275/0x6c0 arch/x86/kvm/ioapic.c:222
>    [...] kvm_ioapic_inject_all arch/x86/kvm/ioapic.c:235
>    [...] kvm_set_ioapic+0x223/0x310 arch/x86/kvm/ioapic.c:670
>    [...] kvm_vm_ioctl_set_irqchip arch/x86/kvm/x86.c:3668
>    [...] kvm_arch_vm_ioctl+0x1a08/0x23c0 arch/x86/kvm/x86.c:3999
>    [...] kvm_vm_ioctl+0x1fa/0x1a70 arch/x86/kvm/../../../virt/kvm/kvm_main.c:3099
>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Cc: stable@vger.kernel.org
> Fixes: af1bae5497b9 ("KVM: x86: bump KVM_MAX_VCPU_ID to 1023")
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Steve Rutherford <srutherford@google.com>
> ---

Yes, the array and the bitmap are always accessed using vcpu_id, so this 
is correct.

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

David