subject:"\[PATCH v2\] KVM\: x86\: fix NULL deref in vcpu_scan

Re: [PATCH v2] KVM: x86: fix NULL deref in vcpu_scan_ioapic

2017-01-12 Thread Paolo Bonzini



On 04/01/2017 10:30, David Hildenbrand wrote:
> Fixes: 5c919412fe61 (kvm/x86: Hyper-V synthetic interrupt controller)
> Cc: sta...@vger.kernel.org # 4.5+

Added both and queued.

Paolo

Re: [PATCH v2] KVM: x86: fix NULL deref in vcpu_scan_ioapic

2017-01-04 Thread David Hildenbrand


Am 04.01.2017 um 03:56 schrieb Wanpeng Li:

From: Wanpeng Li 

Reported by syzkaller:

BUG: unable to handle kernel NULL pointer dereference at 01b0
IP: _raw_spin_lock+0xc/0x30
PGD 3e28eb067
PUD 3f0ac6067
PMD 0
Oops: 0002 [#1] SMP
CPU: 0 PID: 2431 Comm: test Tainted: G   OE   4.10.0-rc1+ #3
Call Trace:
 ? kvm_ioapic_scan_entry+0x3e/0x110 [kvm]
 kvm_arch_vcpu_ioctl_run+0x10a8/0x15f0 [kvm]
 ? pick_next_task_fair+0xe1/0x4e0
 ? kvm_arch_vcpu_load+0xea/0x260 [kvm]
 kvm_vcpu_ioctl+0x33a/0x600 [kvm]
 ? hrtimer_try_to_cancel+0x29/0x130
 ? do_nanosleep+0x97/0xf0
 do_vfs_ioctl+0xa1/0x5d0
 ? __hrtimer_init+0x90/0x90
 ? do_nanosleep+0x5b/0xf0
 SyS_ioctl+0x79/0x90
 do_syscall_64+0x6e/0x180
 entry_SYSCALL64_slow_path+0x25/0x25
RIP: _raw_spin_lock+0xc/0x30 RSP: a43688973cc0

The syzkaller folks reported a NULL pointer dereference that due to
ENABLE_CAP doesn't fail without an irqchip, and synic is activated
which results in a rescan ioapic request has been set although it
should never been set for that vCPU.

#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

#ifndef KVM_CAP_HYPERV_SYNIC
#define KVM_CAP_HYPERV_SYNIC 123
#endif

void* thr(void* arg)
{
struct kvm_enable_cap cap;
cap.flags = 0;
cap.cap = KVM_CAP_HYPERV_SYNIC;
ioctl((long)arg, KVM_ENABLE_CAP, &cap);
return 0;
}

int main()
{
void *host_mem = mmap(0, 0x1000, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
int kvmfd = open("/dev/kvm", 0);
int vmfd = ioctl(kvmfd, KVM_CREATE_VM, 0);
struct kvm_userspace_memory_region memreg;
memreg.slot = 0;
memreg.flags = 0;
memreg.guest_phys_addr = 0;
memreg.memory_size = 0x1000;
memreg.userspace_addr = (unsigned long)host_mem;
memcpy(host_mem,
"\x1a\xb5\x00\x04\x65\x26\x64\x26\xf0\x82\xaa\x00"
"\x02\x00\x3e\x75\x64\xf3\xf3\xf0\x18\x35\x66\xb9"
"\x99\x00\x00\x40\x46\xb8\xc0\x40\x00\x00\x66\xba"
"\x00\x00\x00\x00\x0f\x30\x0f\xfc\xda\xdb\xca\x36"
"\x26\x3e\x67\x3e\x67\xcf\x66\xb9\x4a\x0a\x00\x00"
"\x0f\x32\x2e\x26\x65\x0f\x0f\x01\xa0",
69);
ioctl(vmfd, KVM_SET_USER_MEMORY_REGION, &memreg);
int cpufd = ioctl(vmfd, KVM_CREATE_VCPU, 0);
struct kvm_sregs sregs;
ioctl(cpufd, KVM_GET_SREGS, &sregs);
sregs.cr0 = 0;
sregs.cr4 = 0;
sregs.efer = 0;
sregs.cs.selector = 0;
sregs.cs.base = 0;
ioctl(cpufd, KVM_SET_SREGS, &sregs);
struct kvm_regs regs;
memset(®s, 0, sizeof(regs));
regs.rflags = 2;
ioctl(cpufd, KVM_SET_REGS, ®s);
ioctl(vmfd, KVM_CREATE_IRQCHIP, 0);
pthread_t th;
pthread_create(&th, 0, thr, (void*)(long)cpufd);
usleep(rand() % 1);
ioctl(cpufd, KVM_RUN, 0);
pthread_join(th, 0);
return 0;
}

This patch fix it by failing ENABLE_CAP if without an irqchip.


Not sure if that reproducer should go in there, at least for me
the description of the problem is sufficient.

And I think you should add

Fixes: 5c919412fe61 (kvm/x86: Hyper-V synthetic interrupt controller)
Cc: sta...@vger.kernel.org # 4.5+

I'll put Andrey on cc.

Reviewed-by: David Hildenbrand 



Reported-by: Dmitry Vyukov 
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Cc: Dmitry Vyukov 
Signed-off-by: Wanpeng Li 
---
v1 -> v2:
 * fail ENABLE_CAP if without an irqchip

 arch/x86/kvm/x86.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 5f6e288..2ada44a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3345,6 +3345,8 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu 
*vcpu,

switch (cap->cap) {
case KVM_CAP_HYPERV_SYNIC:
+   if (!irqchip_in_kernel(vcpu->kvm))
+   return -EINVAL;
return kvm_hv_activate_synic(vcpu);
default:
return -EINVAL;




--

David

[PATCH v2] KVM: x86: fix NULL deref in vcpu_scan_ioapic

2017-01-03 Thread Wanpeng Li

From: Wanpeng Li 

Reported by syzkaller:

BUG: unable to handle kernel NULL pointer dereference at 01b0
IP: _raw_spin_lock+0xc/0x30
PGD 3e28eb067
PUD 3f0ac6067
PMD 0
Oops: 0002 [#1] SMP
CPU: 0 PID: 2431 Comm: test Tainted: G   OE   4.10.0-rc1+ #3
Call Trace:
 ? kvm_ioapic_scan_entry+0x3e/0x110 [kvm]
 kvm_arch_vcpu_ioctl_run+0x10a8/0x15f0 [kvm]
 ? pick_next_task_fair+0xe1/0x4e0
 ? kvm_arch_vcpu_load+0xea/0x260 [kvm]
 kvm_vcpu_ioctl+0x33a/0x600 [kvm]
 ? hrtimer_try_to_cancel+0x29/0x130
 ? do_nanosleep+0x97/0xf0
 do_vfs_ioctl+0xa1/0x5d0
 ? __hrtimer_init+0x90/0x90
 ? do_nanosleep+0x5b/0xf0
 SyS_ioctl+0x79/0x90
 do_syscall_64+0x6e/0x180
 entry_SYSCALL64_slow_path+0x25/0x25
RIP: _raw_spin_lock+0xc/0x30 RSP: a43688973cc0

The syzkaller folks reported a NULL pointer dereference that due to 
ENABLE_CAP doesn't fail without an irqchip, and synic is activated 
which results in a rescan ioapic request has been set although it 
should never been set for that vCPU.

#include  
#include  
#include  
#include  
#include  
#include  
#include  
#include  
#include  
#include  

#ifndef KVM_CAP_HYPERV_SYNIC 
#define KVM_CAP_HYPERV_SYNIC 123 
#endif 

void* thr(void* arg) 
{ 
struct kvm_enable_cap cap; 
cap.flags = 0; 
cap.cap = KVM_CAP_HYPERV_SYNIC; 
ioctl((long)arg, KVM_ENABLE_CAP, &cap); 
return 0; 
} 

int main() 
{ 
void *host_mem = mmap(0, 0x1000, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_ANONYMOUS, -1, 0); 
int kvmfd = open("/dev/kvm", 0); 
int vmfd = ioctl(kvmfd, KVM_CREATE_VM, 0); 
struct kvm_userspace_memory_region memreg; 
memreg.slot = 0; 
memreg.flags = 0; 
memreg.guest_phys_addr = 0; 
memreg.memory_size = 0x1000; 
memreg.userspace_addr = (unsigned long)host_mem; 
memcpy(host_mem, 
"\x1a\xb5\x00\x04\x65\x26\x64\x26\xf0\x82\xaa\x00" 
"\x02\x00\x3e\x75\x64\xf3\xf3\xf0\x18\x35\x66\xb9" 
"\x99\x00\x00\x40\x46\xb8\xc0\x40\x00\x00\x66\xba" 
"\x00\x00\x00\x00\x0f\x30\x0f\xfc\xda\xdb\xca\x36" 
"\x26\x3e\x67\x3e\x67\xcf\x66\xb9\x4a\x0a\x00\x00" 
"\x0f\x32\x2e\x26\x65\x0f\x0f\x01\xa0", 
69); 
ioctl(vmfd, KVM_SET_USER_MEMORY_REGION, &memreg); 
int cpufd = ioctl(vmfd, KVM_CREATE_VCPU, 0); 
struct kvm_sregs sregs; 
ioctl(cpufd, KVM_GET_SREGS, &sregs); 
sregs.cr0 = 0; 
sregs.cr4 = 0; 
sregs.efer = 0; 
sregs.cs.selector = 0; 
sregs.cs.base = 0; 
ioctl(cpufd, KVM_SET_SREGS, &sregs); 
struct kvm_regs regs; 
memset(®s, 0, sizeof(regs)); 
regs.rflags = 2; 
ioctl(cpufd, KVM_SET_REGS, ®s); 
ioctl(vmfd, KVM_CREATE_IRQCHIP, 0); 
pthread_t th; 
pthread_create(&th, 0, thr, (void*)(long)cpufd); 
usleep(rand() % 1); 
ioctl(cpufd, KVM_RUN, 0); 
pthread_join(th, 0); 
return 0; 
} 

This patch fix it by failing ENABLE_CAP if without an irqchip.

Reported-by: Dmitry Vyukov 
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Cc: Dmitry Vyukov 
Signed-off-by: Wanpeng Li 
---
v1 -> v2:
 * fail ENABLE_CAP if without an irqchip

 arch/x86/kvm/x86.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 5f6e288..2ada44a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3345,6 +3345,8 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu 
*vcpu,
 
switch (cap->cap) {
case KVM_CAP_HYPERV_SYNIC:
+   if (!irqchip_in_kernel(vcpu->kvm))
+   return -EINVAL;
return kvm_hv_activate_synic(vcpu);
default:
return -EINVAL;
-- 
2.7.4

Re: [PATCH v2] KVM: x86: fix NULL deref in vcpu_scan_ioapic

Re: [PATCH v2] KVM: x86: fix NULL deref in vcpu_scan_ioapic

[PATCH v2] KVM: x86: fix NULL deref in vcpu_scan_ioapic

3 matches

Site Navigation

Mail list logo

Footer information