| From 4f3dbdf47e150016aacd734e663347fcaa768303 Mon Sep 17 00:00:00 2001 |
| From: Wanpeng Li <wanpeng.li@hotmail.com> |
| Date: Thu, 5 Jan 2017 17:39:42 -0800 |
| Subject: KVM: eventfd: fix NULL deref irqbypass consumer |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| From: Wanpeng Li <wanpeng.li@hotmail.com> |
| |
| commit 4f3dbdf47e150016aacd734e663347fcaa768303 upstream. |
| |
| Reported syzkaller: |
| |
| BUG: unable to handle kernel NULL pointer dereference at 0000000000000008 |
| IP: irq_bypass_unregister_consumer+0x9d/0xb70 [irqbypass] |
| PGD 0 |
| |
| Oops: 0002 [#1] SMP |
| CPU: 1 PID: 125 Comm: kworker/1:1 Not tainted 4.9.0+ #1 |
| Workqueue: kvm-irqfd-cleanup irqfd_shutdown [kvm] |
| task: ffff9bbe0dfbb900 task.stack: ffffb61802014000 |
| RIP: 0010:irq_bypass_unregister_consumer+0x9d/0xb70 [irqbypass] |
| Call Trace: |
| irqfd_shutdown+0x66/0xa0 [kvm] |
| process_one_work+0x16b/0x480 |
| worker_thread+0x4b/0x500 |
| kthread+0x101/0x140 |
| ? process_one_work+0x480/0x480 |
| ? kthread_create_on_node+0x60/0x60 |
| ret_from_fork+0x25/0x30 |
| RIP: irq_bypass_unregister_consumer+0x9d/0xb70 [irqbypass] RSP: ffffb61802017e20 |
| CR2: 0000000000000008 |
| |
| The syzkaller folks reported a NULL pointer dereference that due to |
| unregister an consumer which fails registration before. The syzkaller |
| creates two VMs w/ an equal eventfd occasionally. So the second VM |
| fails to register an irqbypass consumer. It will make irqfd as inactive |
| and queue an workqueue work to shutdown irqfd and unregister the irqbypass |
| consumer when eventfd is closed. However, the second consumer has been |
| initialized though it fails registration. So the token(same as the first |
| VM's) is taken to unregister the consumer through the workqueue, the |
| consumer of the first VM is found and unregistered, then NULL deref incurred |
| in the path of deleting consumer from the consumers list. |
| |
| This patch fixes it by making irq_bypass_register/unregister_consumer() |
| looks for the consumer entry based on consumer pointer itself instead of |
| token matching. |
| |
| Reported-by: Dmitry Vyukov <dvyukov@google.com> |
| Suggested-by: Alex Williamson <alex.williamson@redhat.com> |
| Cc: Paolo Bonzini <pbonzini@redhat.com> |
| Cc: Radim Krčmář <rkrcmar@redhat.com> |
| Cc: Dmitry Vyukov <dvyukov@google.com> |
| Cc: Alex Williamson <alex.williamson@redhat.com> |
| Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com> |
| Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| virt/lib/irqbypass.c | 4 ++-- |
| 1 file changed, 2 insertions(+), 2 deletions(-) |
| |
| --- a/virt/lib/irqbypass.c |
| +++ b/virt/lib/irqbypass.c |
| @@ -188,7 +188,7 @@ int irq_bypass_register_consumer(struct |
| mutex_lock(&lock); |
| |
| list_for_each_entry(tmp, &consumers, node) { |
| - if (tmp->token == consumer->token) { |
| + if (tmp->token == consumer->token || tmp == consumer) { |
| mutex_unlock(&lock); |
| module_put(THIS_MODULE); |
| return -EBUSY; |
| @@ -235,7 +235,7 @@ void irq_bypass_unregister_consumer(stru |
| mutex_lock(&lock); |
| |
| list_for_each_entry(tmp, &consumers, node) { |
| - if (tmp->token != consumer->token) |
| + if (tmp != consumer) |
| continue; |
| |
| list_for_each_entry(producer, &producers, node) { |