* [PATCH 1/7] kvm: x86: avoid warning on repeated KVM_SET_TSS_ADDR
2016-06-01 12:09 [PATCH 0/7] KVM: syzkaller fixes Paolo Bonzini
@ 2016-06-01 12:09 ` Paolo Bonzini
2016-06-01 12:09 ` [PATCH 2/7] KVM: x86: avoid vmalloc(0) in the KVM_SET_CPUID Paolo Bonzini
` (6 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2016-06-01 12:09 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: rkrcmar
Found by syzkaller:
WARNING: CPU: 3 PID: 15175 at arch/x86/kvm/x86.c:7705 __x86_set_memory_region+0x1dc/0x1f0 [kvm]()
CPU: 3 PID: 15175 Comm: a.out Tainted: G W 4.4.6-300.fc23.x86_64 #1
Hardware name: LENOVO 2325F51/2325F51, BIOS G2ET32WW (1.12 ) 05/30/2012
0000000000000286 00000000950899a7 ffff88011ab3fbf0 ffffffff813b542e
0000000000000000 ffffffffa0966496 ffff88011ab3fc28 ffffffff810a40f2
00000000000001fd 0000000000003000 ffff88014fc50000 0000000000000000
Call Trace:
[<ffffffff813b542e>] dump_stack+0x63/0x85
[<ffffffff810a40f2>] warn_slowpath_common+0x82/0xc0
[<ffffffff810a423a>] warn_slowpath_null+0x1a/0x20
[<ffffffffa09251cc>] __x86_set_memory_region+0x1dc/0x1f0 [kvm]
[<ffffffffa092521b>] x86_set_memory_region+0x3b/0x60 [kvm]
[<ffffffffa09bb61c>] vmx_set_tss_addr+0x3c/0x150 [kvm_intel]
[<ffffffffa092f4d4>] kvm_arch_vm_ioctl+0x654/0xbc0 [kvm]
[<ffffffffa091d31a>] kvm_vm_ioctl+0x9a/0x6f0 [kvm]
[<ffffffff81241248>] do_vfs_ioctl+0x298/0x480
[<ffffffff812414a9>] SyS_ioctl+0x79/0x90
[<ffffffff817a04ee>] entry_SYSCALL_64_fastpath+0x12/0x71
Testcase:
#include <unistd.h>
#include <sys/ioctl.h>
#include <fcntl.h>
#include <string.h>
#include <linux/kvm.h>
long r[8];
int main()
{
memset(r, -1, sizeof(r));
r[2] = open("/dev/kvm", O_RDONLY|O_TRUNC);
r[3] = ioctl(r[2], KVM_CREATE_VM, 0x0ul);
r[5] = ioctl(r[3], KVM_SET_TSS_ADDR, 0x20000000ul);
r[7] = ioctl(r[3], KVM_SET_TSS_ADDR, 0x20000000ul);
return 0;
}
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/x86.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 199a87c20a98..6c9793c64522 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7828,7 +7828,7 @@ int __x86_set_memory_region(struct kvm *kvm, int id, gpa_t gpa, u32 size)
slot = id_to_memslot(slots, id);
if (size) {
- if (WARN_ON(slot->npages))
+ if (slot->npages)
return -EEXIST;
/*
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/7] KVM: x86: avoid vmalloc(0) in the KVM_SET_CPUID
2016-06-01 12:09 [PATCH 0/7] KVM: syzkaller fixes Paolo Bonzini
2016-06-01 12:09 ` [PATCH 1/7] kvm: x86: avoid warning on repeated KVM_SET_TSS_ADDR Paolo Bonzini
@ 2016-06-01 12:09 ` Paolo Bonzini
2016-06-01 12:09 ` [PATCH 3/7] KVM: fail KVM_SET_VCPU_EVENTS with invalid exception number Paolo Bonzini
` (5 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2016-06-01 12:09 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: rkrcmar
This causes an ugly dmesg splat. Beautified syzkaller testcase:
#include <unistd.h>
#include <sys/syscall.h>
#include <sys/ioctl.h>
#include <fcntl.h>
#include <linux/kvm.h>
long r[8];
int main()
{
struct kvm_cpuid2 c = { 0 };
r[2] = open("/dev/kvm", O_RDWR);
r[3] = ioctl(r[2], KVM_CREATE_VM, 0);
r[4] = ioctl(r[3], KVM_CREATE_VCPU, 0x8);
r[7] = ioctl(r[4], KVM_SET_CPUID, &c);
return 0;
}
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/cpuid.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 769af907f824..7597b42a8a88 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -181,19 +181,22 @@ int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu *vcpu,
struct kvm_cpuid_entry __user *entries)
{
int r, i;
- struct kvm_cpuid_entry *cpuid_entries;
+ struct kvm_cpuid_entry *cpuid_entries = NULL;
r = -E2BIG;
if (cpuid->nent > KVM_MAX_CPUID_ENTRIES)
goto out;
r = -ENOMEM;
- cpuid_entries = vmalloc(sizeof(struct kvm_cpuid_entry) * cpuid->nent);
- if (!cpuid_entries)
- goto out;
- r = -EFAULT;
- if (copy_from_user(cpuid_entries, entries,
- cpuid->nent * sizeof(struct kvm_cpuid_entry)))
- goto out_free;
+ if (cpuid->nent) {
+ cpuid_entries = vmalloc(sizeof(struct kvm_cpuid_entry) *
+ cpuid->nent);
+ if (!cpuid_entries)
+ goto out;
+ r = -EFAULT;
+ if (copy_from_user(cpuid_entries, entries,
+ cpuid->nent * sizeof(struct kvm_cpuid_entry)))
+ goto out;
+ }
for (i = 0; i < cpuid->nent; i++) {
vcpu->arch.cpuid_entries[i].function = cpuid_entries[i].function;
vcpu->arch.cpuid_entries[i].eax = cpuid_entries[i].eax;
@@ -212,9 +215,8 @@ int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu *vcpu,
kvm_x86_ops->cpuid_update(vcpu);
r = kvm_update_cpuid(vcpu);
-out_free:
- vfree(cpuid_entries);
out:
+ vfree(cpuid_entries);
return r;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/7] KVM: fail KVM_SET_VCPU_EVENTS with invalid exception number
2016-06-01 12:09 [PATCH 0/7] KVM: syzkaller fixes Paolo Bonzini
2016-06-01 12:09 ` [PATCH 1/7] kvm: x86: avoid warning on repeated KVM_SET_TSS_ADDR Paolo Bonzini
2016-06-01 12:09 ` [PATCH 2/7] KVM: x86: avoid vmalloc(0) in the KVM_SET_CPUID Paolo Bonzini
@ 2016-06-01 12:09 ` Paolo Bonzini
2016-06-01 12:09 ` [PATCH 4/7] KVM: irqfd: fix NULL pointer dereference in kvm_irq_map_gsi Paolo Bonzini
` (4 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2016-06-01 12:09 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: rkrcmar
This cannot be returned by KVM_GET_VCPU_EVENTS, so it is okay to return
EINVAL. It causes a WARN from exception_type:
WARNING: CPU: 3 PID: 16732 at arch/x86/kvm/x86.c:345 exception_type+0x49/0x50 [kvm]()
CPU: 3 PID: 16732 Comm: a.out Tainted: G W 4.4.6-300.fc23.x86_64 #1
Hardware name: LENOVO 2325F51/2325F51, BIOS G2ET32WW (1.12 ) 05/30/2012
0000000000000286 000000006308a48b ffff8800bec7fcf8 ffffffff813b542e
0000000000000000 ffffffffa0966496 ffff8800bec7fd30 ffffffff810a40f2
ffff8800552a8000 0000000000000000 00000000002c267c 0000000000000001
Call Trace:
[<ffffffff813b542e>] dump_stack+0x63/0x85
[<ffffffff810a40f2>] warn_slowpath_common+0x82/0xc0
[<ffffffff810a423a>] warn_slowpath_null+0x1a/0x20
[<ffffffffa0924809>] exception_type+0x49/0x50 [kvm]
[<ffffffffa0934622>] kvm_arch_vcpu_ioctl_run+0x10a2/0x14e0 [kvm]
[<ffffffffa091c04d>] kvm_vcpu_ioctl+0x33d/0x620 [kvm]
[<ffffffff81241248>] do_vfs_ioctl+0x298/0x480
[<ffffffff812414a9>] SyS_ioctl+0x79/0x90
[<ffffffff817a04ee>] entry_SYSCALL_64_fastpath+0x12/0x71
---[ end trace b1a0391266848f50 ]---
Testcase (beautified/reduced from syzkaller output):
#include <unistd.h>
#include <sys/syscall.h>
#include <string.h>
#include <stdint.h>
#include <fcntl.h>
#include <sys/ioctl.h>
#include <linux/kvm.h>
long r[31];
int main()
{
memset(r, -1, sizeof(r));
r[2] = open("/dev/kvm", O_RDONLY);
r[3] = ioctl(r[2], KVM_CREATE_VM, 0);
r[7] = ioctl(r[3], KVM_CREATE_VCPU, 0);
struct kvm_vcpu_events ve = {
.exception.injected = 1,
.exception.nr = 0xd4
};
r[27] = ioctl(r[7], KVM_SET_VCPU_EVENTS, &ve);
r[30] = ioctl(r[7], KVM_RUN, 0);
return 0;
}
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/x86.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 6c9793c64522..71284533f0b7 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2973,6 +2973,10 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
| KVM_VCPUEVENT_VALID_SMM))
return -EINVAL;
+ if (events->exception.injected &&
+ (events->exception.nr > 31 || events->exception.nr == NMI_VECTOR))
+ return -EINVAL;
+
process_nmi(vcpu);
vcpu->arch.exception.pending = events->exception.injected;
vcpu->arch.exception.nr = events->exception.nr;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/7] KVM: irqfd: fix NULL pointer dereference in kvm_irq_map_gsi
2016-06-01 12:09 [PATCH 0/7] KVM: syzkaller fixes Paolo Bonzini
` (2 preceding siblings ...)
2016-06-01 12:09 ` [PATCH 3/7] KVM: fail KVM_SET_VCPU_EVENTS with invalid exception number Paolo Bonzini
@ 2016-06-01 12:09 ` Paolo Bonzini
2017-01-09 22:39 ` Steve Rutherford
2016-06-01 12:09 ` [PATCH 5/7] KVM: x86: avoid vmalloc(0) in the KVM_SET_CPUID Paolo Bonzini
` (3 subsequent siblings)
7 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2016-06-01 12:09 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: rkrcmar, stable
Found by syzkaller:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000120
IP: [<ffffffffa0797202>] kvm_irq_map_gsi+0x12/0x90 [kvm]
PGD 6f80b067 PUD b6535067 PMD 0
Oops: 0000 [#1] SMP
CPU: 3 PID: 4988 Comm: a.out Not tainted 4.4.9-300.fc23.x86_64 #1
[...]
Call Trace:
[<ffffffffa0795f62>] irqfd_update+0x32/0xc0 [kvm]
[<ffffffffa0796c7c>] kvm_irqfd+0x3dc/0x5b0 [kvm]
[<ffffffffa07943f4>] kvm_vm_ioctl+0x164/0x6f0 [kvm]
[<ffffffff81241648>] do_vfs_ioctl+0x298/0x480
[<ffffffff812418a9>] SyS_ioctl+0x79/0x90
[<ffffffff817a1062>] tracesys_phase2+0x84/0x89
Code: b5 71 a7 e0 5b 41 5c 41 5d 5d f3 c3 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 48 8b 8f 10 2e 00 00 31 c0 48 89 e5 <39> 91 20 01 00 00 76 6a 48 63 d2 48 8b 94 d1 28 01 00 00 48 85
RIP [<ffffffffa0797202>] kvm_irq_map_gsi+0x12/0x90 [kvm]
RSP <ffff8800926cbca8>
CR2: 0000000000000120
Testcase:
#include <unistd.h>
#include <sys/syscall.h>
#include <string.h>
#include <stdint.h>
#include <linux/kvm.h>
#include <fcntl.h>
#include <sys/ioctl.h>
long r[26];
int main()
{
memset(r, -1, sizeof(r));
r[2] = open("/dev/kvm", 0);
r[3] = ioctl(r[2], KVM_CREATE_VM, 0);
struct kvm_irqfd ifd;
ifd.fd = syscall(SYS_eventfd2, 5, 0);
ifd.gsi = 3;
ifd.flags = 2;
ifd.resamplefd = ifd.fd;
r[25] = ioctl(r[3], KVM_IRQFD, &ifd);
return 0;
}
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
virt/kvm/irqchip.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
index fe84e1a95dd5..8db197bb6c7a 100644
--- a/virt/kvm/irqchip.c
+++ b/virt/kvm/irqchip.c
@@ -40,7 +40,7 @@ int kvm_irq_map_gsi(struct kvm *kvm,
irq_rt = srcu_dereference_check(kvm->irq_routing, &kvm->irq_srcu,
lockdep_is_held(&kvm->irq_lock));
- if (gsi < irq_rt->nr_rt_entries) {
+ if (irq_rt && gsi < irq_rt->nr_rt_entries) {
hlist_for_each_entry(e, &irq_rt->map[gsi], link) {
entries[n] = *e;
++n;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 4/7] KVM: irqfd: fix NULL pointer dereference in kvm_irq_map_gsi
2016-06-01 12:09 ` [PATCH 4/7] KVM: irqfd: fix NULL pointer dereference in kvm_irq_map_gsi Paolo Bonzini
@ 2017-01-09 22:39 ` Steve Rutherford
2017-01-10 9:32 ` Paolo Bonzini
0 siblings, 1 reply; 12+ messages in thread
From: Steve Rutherford @ 2017-01-09 22:39 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: KVM list, Radim Krčmář
Doesn't this same bug also affect kvm_irq_map_chip_pin?
(Note: I haven't chased down whether anything can even reach here if
irq_rt is NULL.)
int kvm_irq_map_chip_pin(struct kvm *kvm, unsigned irqchip, unsigned pin)
{
struct kvm_irq_routing_table *irq_rt;
irq_rt = srcu_dereference(kvm->irq_routing, &kvm->irq_srcu);
+
+ if (!irq_rt)
+ return -1;
+
return irq_rt->chip[irqchip][pin];
}
Steve
On Wed, Jun 1, 2016 at 5:09 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Found by syzkaller:
>
> BUG: unable to handle kernel NULL pointer dereference at 0000000000000120
> IP: [<ffffffffa0797202>] kvm_irq_map_gsi+0x12/0x90 [kvm]
> PGD 6f80b067 PUD b6535067 PMD 0
> Oops: 0000 [#1] SMP
> CPU: 3 PID: 4988 Comm: a.out Not tainted 4.4.9-300.fc23.x86_64 #1
> [...]
> Call Trace:
> [<ffffffffa0795f62>] irqfd_update+0x32/0xc0 [kvm]
> [<ffffffffa0796c7c>] kvm_irqfd+0x3dc/0x5b0 [kvm]
> [<ffffffffa07943f4>] kvm_vm_ioctl+0x164/0x6f0 [kvm]
> [<ffffffff81241648>] do_vfs_ioctl+0x298/0x480
> [<ffffffff812418a9>] SyS_ioctl+0x79/0x90
> [<ffffffff817a1062>] tracesys_phase2+0x84/0x89
> Code: b5 71 a7 e0 5b 41 5c 41 5d 5d f3 c3 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 48 8b 8f 10 2e 00 00 31 c0 48 89 e5 <39> 91 20 01 00 00 76 6a 48 63 d2 48 8b 94 d1 28 01 00 00 48 85
> RIP [<ffffffffa0797202>] kvm_irq_map_gsi+0x12/0x90 [kvm]
> RSP <ffff8800926cbca8>
> CR2: 0000000000000120
>
> Testcase:
>
> #include <unistd.h>
> #include <sys/syscall.h>
> #include <string.h>
> #include <stdint.h>
> #include <linux/kvm.h>
> #include <fcntl.h>
> #include <sys/ioctl.h>
>
> long r[26];
>
> int main()
> {
> memset(r, -1, sizeof(r));
> r[2] = open("/dev/kvm", 0);
> r[3] = ioctl(r[2], KVM_CREATE_VM, 0);
>
> struct kvm_irqfd ifd;
> ifd.fd = syscall(SYS_eventfd2, 5, 0);
> ifd.gsi = 3;
> ifd.flags = 2;
> ifd.resamplefd = ifd.fd;
> r[25] = ioctl(r[3], KVM_IRQFD, &ifd);
> return 0;
> }
>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> virt/kvm/irqchip.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
> index fe84e1a95dd5..8db197bb6c7a 100644
> --- a/virt/kvm/irqchip.c
> +++ b/virt/kvm/irqchip.c
> @@ -40,7 +40,7 @@ int kvm_irq_map_gsi(struct kvm *kvm,
>
> irq_rt = srcu_dereference_check(kvm->irq_routing, &kvm->irq_srcu,
> lockdep_is_held(&kvm->irq_lock));
> - if (gsi < irq_rt->nr_rt_entries) {
> + if (irq_rt && gsi < irq_rt->nr_rt_entries) {
> hlist_for_each_entry(e, &irq_rt->map[gsi], link) {
> entries[n] = *e;
> ++n;
> --
> 1.8.3.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 4/7] KVM: irqfd: fix NULL pointer dereference in kvm_irq_map_gsi
2017-01-09 22:39 ` Steve Rutherford
@ 2017-01-10 9:32 ` Paolo Bonzini
0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2017-01-10 9:32 UTC (permalink / raw)
To: Steve Rutherford; +Cc: KVM list, Radim Krčmář
> Doesn't this same bug also affect kvm_irq_map_chip_pin?
>
> (Note: I haven't chased down whether anything can even reach here if
> irq_rt is NULL.)
No, it is safe because kvm_irq_map_chip_pin requires having created an
irqchip, and:
- host code (kvm_set_irq) in virt/kvm/irqchip.c goes through kvm_irq_map_gsi,
and until you have a successful kvm_irq_map_gsi you cannot get to
kvm_notify_acked_irq
- guest code (such as ioapic_write_indirect) can call kvm_irq_map_chip_pin,
but it cannot run until KVM_CREATE_IRQCHIP returns. KVM_CREATE_IRQCHIP
requires that there are no VCPUs, and kvm->lock protects both KVM_CREATE_IRQCHIP
and (the first part of) KVM_CREATE_VCPU.
Maybe it was possible, but very unlikely, before commit 557abc40d121
("KVM: remove kvm_vcpu_compatible", 2016-06-16).
Paolo
> int kvm_irq_map_chip_pin(struct kvm *kvm, unsigned irqchip, unsigned pin)
> {
> struct kvm_irq_routing_table *irq_rt;
>
> irq_rt = srcu_dereference(kvm->irq_routing, &kvm->irq_srcu);
> +
> + if (!irq_rt)
> + return -1;
> +
> return irq_rt->chip[irqchip][pin];
> }
>
> Steve
>
> On Wed, Jun 1, 2016 at 5:09 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > Found by syzkaller:
> >
> > BUG: unable to handle kernel NULL pointer dereference at
> > 0000000000000120
> > IP: [<ffffffffa0797202>] kvm_irq_map_gsi+0x12/0x90 [kvm]
> > PGD 6f80b067 PUD b6535067 PMD 0
> > Oops: 0000 [#1] SMP
> > CPU: 3 PID: 4988 Comm: a.out Not tainted 4.4.9-300.fc23.x86_64 #1
> > [...]
> > Call Trace:
> > [<ffffffffa0795f62>] irqfd_update+0x32/0xc0 [kvm]
> > [<ffffffffa0796c7c>] kvm_irqfd+0x3dc/0x5b0 [kvm]
> > [<ffffffffa07943f4>] kvm_vm_ioctl+0x164/0x6f0 [kvm]
> > [<ffffffff81241648>] do_vfs_ioctl+0x298/0x480
> > [<ffffffff812418a9>] SyS_ioctl+0x79/0x90
> > [<ffffffff817a1062>] tracesys_phase2+0x84/0x89
> > Code: b5 71 a7 e0 5b 41 5c 41 5d 5d f3 c3 66 66 66 66 2e 0f 1f 84 00 00
> > 00 00 00 0f 1f 44 00 00 55 48 8b 8f 10 2e 00 00 31 c0 48 89 e5 <39> 91
> > 20 01 00 00 76 6a 48 63 d2 48 8b 94 d1 28 01 00 00 48 85
> > RIP [<ffffffffa0797202>] kvm_irq_map_gsi+0x12/0x90 [kvm]
> > RSP <ffff8800926cbca8>
> > CR2: 0000000000000120
> >
> > Testcase:
> >
> > #include <unistd.h>
> > #include <sys/syscall.h>
> > #include <string.h>
> > #include <stdint.h>
> > #include <linux/kvm.h>
> > #include <fcntl.h>
> > #include <sys/ioctl.h>
> >
> > long r[26];
> >
> > int main()
> > {
> > memset(r, -1, sizeof(r));
> > r[2] = open("/dev/kvm", 0);
> > r[3] = ioctl(r[2], KVM_CREATE_VM, 0);
> >
> > struct kvm_irqfd ifd;
> > ifd.fd = syscall(SYS_eventfd2, 5, 0);
> > ifd.gsi = 3;
> > ifd.flags = 2;
> > ifd.resamplefd = ifd.fd;
> > r[25] = ioctl(r[3], KVM_IRQFD, &ifd);
> > return 0;
> > }
> >
> > Reported-by: Dmitry Vyukov <dvyukov@google.com>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> > virt/kvm/irqchip.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
> > index fe84e1a95dd5..8db197bb6c7a 100644
> > --- a/virt/kvm/irqchip.c
> > +++ b/virt/kvm/irqchip.c
> > @@ -40,7 +40,7 @@ int kvm_irq_map_gsi(struct kvm *kvm,
> >
> > irq_rt = srcu_dereference_check(kvm->irq_routing, &kvm->irq_srcu,
> > lockdep_is_held(&kvm->irq_lock));
> > - if (gsi < irq_rt->nr_rt_entries) {
> > + if (irq_rt && gsi < irq_rt->nr_rt_entries) {
> > hlist_for_each_entry(e, &irq_rt->map[gsi], link) {
> > entries[n] = *e;
> > ++n;
> > --
> > 1.8.3.1
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 5/7] KVM: x86: avoid vmalloc(0) in the KVM_SET_CPUID
2016-06-01 12:09 [PATCH 0/7] KVM: syzkaller fixes Paolo Bonzini
` (3 preceding siblings ...)
2016-06-01 12:09 ` [PATCH 4/7] KVM: irqfd: fix NULL pointer dereference in kvm_irq_map_gsi Paolo Bonzini
@ 2016-06-01 12:09 ` Paolo Bonzini
2016-06-04 23:55 ` Wanpeng Li
2016-06-01 12:09 ` [PATCH 6/7] KVM: x86: fix OOPS after invalid KVM_SET_DEBUGREGS Paolo Bonzini
` (2 subsequent siblings)
7 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2016-06-01 12:09 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: rkrcmar
This causes an ugly dmesg splat. Beautified syzkaller testcase:
#include <unistd.h>
#include <sys/syscall.h>
#include <sys/ioctl.h>
#include <fcntl.h>
#include <linux/kvm.h>
long r[8];
int main()
{
struct kvm_irq_routing ir = { 0 };
r[2] = open("/dev/kvm", O_RDWR);
r[3] = ioctl(r[2], KVM_CREATE_VM, 0);
r[4] = ioctl(r[3], KVM_SET_GSI_ROUTING, &ir);
return 0;
}
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
virt/kvm/kvm_main.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 37af23052470..02e98f3131bd 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2935,7 +2935,7 @@ static long kvm_vm_ioctl(struct file *filp,
case KVM_SET_GSI_ROUTING: {
struct kvm_irq_routing routing;
struct kvm_irq_routing __user *urouting;
- struct kvm_irq_routing_entry *entries;
+ struct kvm_irq_routing_entry *entries = NULL;
r = -EFAULT;
if (copy_from_user(&routing, argp, sizeof(routing)))
@@ -2945,15 +2945,17 @@ static long kvm_vm_ioctl(struct file *filp,
goto out;
if (routing.flags)
goto out;
- r = -ENOMEM;
- entries = vmalloc(routing.nr * sizeof(*entries));
- if (!entries)
- goto out;
- r = -EFAULT;
- urouting = argp;
- if (copy_from_user(entries, urouting->entries,
- routing.nr * sizeof(*entries)))
- goto out_free_irq_routing;
+ if (routing.nr) {
+ r = -ENOMEM;
+ entries = vmalloc(routing.nr * sizeof(*entries));
+ if (!entries)
+ goto out;
+ r = -EFAULT;
+ urouting = argp;
+ if (copy_from_user(entries, urouting->entries,
+ routing.nr * sizeof(*entries)))
+ goto out_free_irq_routing;
+ }
r = kvm_set_irq_routing(kvm, entries, routing.nr,
routing.flags);
out_free_irq_routing:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 5/7] KVM: x86: avoid vmalloc(0) in the KVM_SET_CPUID
2016-06-01 12:09 ` [PATCH 5/7] KVM: x86: avoid vmalloc(0) in the KVM_SET_CPUID Paolo Bonzini
@ 2016-06-04 23:55 ` Wanpeng Li
0 siblings, 0 replies; 12+ messages in thread
From: Wanpeng Li @ 2016-06-04 23:55 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: linux-kernel, kvm, Radim Krcmar
2016-06-01 20:09 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
> This causes an ugly dmesg splat. Beautified syzkaller testcase:
>
> #include <unistd.h>
> #include <sys/syscall.h>
> #include <sys/ioctl.h>
> #include <fcntl.h>
> #include <linux/kvm.h>
>
> long r[8];
>
> int main()
> {
> struct kvm_irq_routing ir = { 0 };
> r[2] = open("/dev/kvm", O_RDWR);
> r[3] = ioctl(r[2], KVM_CREATE_VM, 0);
> r[4] = ioctl(r[3], KVM_SET_GSI_ROUTING, &ir);
> return 0;
> }
>
The patch subject is not correct.
Regards,
Wanpeng Li
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 6/7] KVM: x86: fix OOPS after invalid KVM_SET_DEBUGREGS
2016-06-01 12:09 [PATCH 0/7] KVM: syzkaller fixes Paolo Bonzini
` (4 preceding siblings ...)
2016-06-01 12:09 ` [PATCH 5/7] KVM: x86: avoid vmalloc(0) in the KVM_SET_CPUID Paolo Bonzini
@ 2016-06-01 12:09 ` Paolo Bonzini
2016-06-01 12:09 ` [PATCH 7/7] KVM: x86: protect KVM_CREATE_PIT/KVM_CREATE_PIT2 with kvm->lock Paolo Bonzini
2016-06-01 16:07 ` [PATCH 0/7] KVM: syzkaller fixes Radim Krčmář
7 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2016-06-01 12:09 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: rkrcmar, stable
MOV to DR6 or DR7 causes a #GP if an attempt is made to write a 1 to
any of bits 63:32. However, this is not detected at KVM_SET_DEBUGREGS
time, and the next KVM_RUN oopses:
general protection fault: 0000 [#1] SMP
CPU: 2 PID: 14987 Comm: a.out Not tainted 4.4.9-300.fc23.x86_64 #1
Hardware name: LENOVO 2325F51/2325F51, BIOS G2ET32WW (1.12 ) 05/30/2012
[...]
Call Trace:
[<ffffffffa072c93d>] kvm_arch_vcpu_ioctl_run+0x141d/0x14e0 [kvm]
[<ffffffffa071405d>] kvm_vcpu_ioctl+0x33d/0x620 [kvm]
[<ffffffff81241648>] do_vfs_ioctl+0x298/0x480
[<ffffffff812418a9>] SyS_ioctl+0x79/0x90
[<ffffffff817a0f2e>] entry_SYSCALL_64_fastpath+0x12/0x71
Code: 55 83 ff 07 48 89 e5 77 27 89 ff ff 24 fd 90 87 80 81 0f 23 fe 5d c3 0f 23 c6 5d c3 0f 23 ce 5d c3 0f 23 d6 5d c3 0f 23 de 5d c3 <0f> 23 f6 5d c3 0f 0b 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00
RIP [<ffffffff810639eb>] native_set_debugreg+0x2b/0x40
RSP <ffff88005836bd50>
Testcase (beautified/reduced from syzkaller output):
#include <unistd.h>
#include <sys/syscall.h>
#include <string.h>
#include <stdint.h>
#include <linux/kvm.h>
#include <fcntl.h>
#include <sys/ioctl.h>
long r[8];
int main()
{
struct kvm_debugregs dr = { 0 };
r[2] = open("/dev/kvm", O_RDONLY);
r[3] = ioctl(r[2], KVM_CREATE_VM, 0);
r[4] = ioctl(r[3], KVM_CREATE_VCPU, 7);
memcpy(&dr,
"\x5d\x6a\x6b\xe8\x57\x3b\x4b\x7e\xcf\x0d\xa1\x72"
"\xa3\x4a\x29\x0c\xfc\x6d\x44\x00\xa7\x52\xc7\xd8"
"\x00\xdb\x89\x9d\x78\xb5\x54\x6b\x6b\x13\x1c\xe9"
"\x5e\xd3\x0e\x40\x6f\xb4\x66\xf7\x5b\xe3\x36\xcb",
48);
r[7] = ioctl(r[4], KVM_SET_DEBUGREGS, &dr);
r[6] = ioctl(r[4], KVM_RUN, 0);
}
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/x86.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 71284533f0b7..3fbf88f85a2d 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3041,6 +3041,11 @@ static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu,
if (dbgregs->flags)
return -EINVAL;
+ if (dbgregs->dr6 & ~0xffffffffull)
+ return -EINVAL;
+ if (dbgregs->dr7 & ~0xffffffffull)
+ return -EINVAL;
+
memcpy(vcpu->arch.db, dbgregs->db, sizeof(vcpu->arch.db));
kvm_update_dr0123(vcpu);
vcpu->arch.dr6 = dbgregs->dr6;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 7/7] KVM: x86: protect KVM_CREATE_PIT/KVM_CREATE_PIT2 with kvm->lock
2016-06-01 12:09 [PATCH 0/7] KVM: syzkaller fixes Paolo Bonzini
` (5 preceding siblings ...)
2016-06-01 12:09 ` [PATCH 6/7] KVM: x86: fix OOPS after invalid KVM_SET_DEBUGREGS Paolo Bonzini
@ 2016-06-01 12:09 ` Paolo Bonzini
2016-06-01 16:07 ` [PATCH 0/7] KVM: syzkaller fixes Radim Krčmář
7 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2016-06-01 12:09 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: rkrcmar
The syzkaller folks reported a NULL pointer dereference that seems
to be cause by a race between KVM_CREATE_IRQCHIP and KVM_CREATE_PIT2.
The former takes kvm->lock (except when registering the devices,
which needs kvm->slots_lock); the latter takes kvm->slots_lock only.
Change KVM_CREATE_PIT2 to follow the same model as KVM_CREATE_IRQCHIP.
Testcase:
#include <pthread.h>
#include <linux/kvm.h>
#include <fcntl.h>
#include <sys/ioctl.h>
#include <stdint.h>
#include <string.h>
#include <stdlib.h>
#include <sys/syscall.h>
#include <unistd.h>
long r[23];
void* thr1(void* arg)
{
struct kvm_pit_config pitcfg = { .flags = 4 };
switch ((long)arg) {
case 0: r[2] = open("/dev/kvm", O_RDONLY|O_ASYNC); break;
case 1: r[3] = ioctl(r[2], KVM_CREATE_VM, 0); break;
case 2: r[4] = ioctl(r[3], KVM_CREATE_IRQCHIP, 0); break;
case 3: r[22] = ioctl(r[3], KVM_CREATE_PIT2, &pitcfg); break;
}
return 0;
}
int main(int argc, char **argv)
{
long i;
pthread_t th[4];
memset(r, -1, sizeof(r));
for (i = 0; i < 4; i++) {
pthread_create(&th[i], 0, thr, (void*)i);
if (argc > 1 && rand()%2) usleep(rand()%1000);
}
usleep(20000);
return 0;
}
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/i8254.c | 4 +++-
arch/x86/kvm/x86.c | 4 ++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index a4bf5b45d65a..5fb6c620180e 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -645,7 +645,6 @@ static const struct kvm_io_device_ops speaker_dev_ops = {
.write = speaker_ioport_write,
};
-/* Caller must hold slots_lock */
struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
{
struct kvm_pit *pit;
@@ -690,6 +689,7 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
kvm_pit_set_reinject(pit, true);
+ mutex_lock(&kvm->slots_lock);
kvm_iodevice_init(&pit->dev, &pit_dev_ops);
ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, KVM_PIT_BASE_ADDRESS,
KVM_PIT_MEM_LENGTH, &pit->dev);
@@ -704,12 +704,14 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
if (ret < 0)
goto fail_register_speaker;
}
+ mutex_unlock(&kvm->slots_lock);
return pit;
fail_register_speaker:
kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &pit->dev);
fail_register_pit:
+ mutex_unlock(&kvm->slots_lock);
kvm_pit_set_reinject(pit, false);
kthread_stop(pit->worker_task);
fail_kthread:
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 3fbf88f85a2d..99bb0cf93747 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3878,7 +3878,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
sizeof(struct kvm_pit_config)))
goto out;
create_pit:
- mutex_lock(&kvm->slots_lock);
+ mutex_lock(&kvm->lock);
r = -EEXIST;
if (kvm->arch.vpit)
goto create_pit_unlock;
@@ -3887,7 +3887,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
if (kvm->arch.vpit)
r = 0;
create_pit_unlock:
- mutex_unlock(&kvm->slots_lock);
+ mutex_unlock(&kvm->lock);
break;
case KVM_GET_IRQCHIP: {
/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 0/7] KVM: syzkaller fixes
2016-06-01 12:09 [PATCH 0/7] KVM: syzkaller fixes Paolo Bonzini
` (6 preceding siblings ...)
2016-06-01 12:09 ` [PATCH 7/7] KVM: x86: protect KVM_CREATE_PIT/KVM_CREATE_PIT2 with kvm->lock Paolo Bonzini
@ 2016-06-01 16:07 ` Radim Krčmář
7 siblings, 0 replies; 12+ messages in thread
From: Radim Krčmář @ 2016-06-01 16:07 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: linux-kernel, kvm
2016-06-01 14:09+0200, Paolo Bonzini:
> These fix most of the bugs reported by Dmitry Vyukov a while back.
> I couldn't reproduce one of the bugs (patch 7) but the fix is easy.
> Probably, more VM ioctls should take kvm->lock, but I have not looked
> at it yet.
Applied, thanks.
> I have only marked for stable the two patches that fix an oops. However,
> all of patches 1-6 could go in 4.7-rc, I think.
Normal userspaces don't exercise modified paths, so I'll prepare [1-6/7]
for -rc1, as thorough testing wouldn't make a difference ...
^ permalink raw reply [flat|nested] 12+ messages in thread