On 05/01/2017 10:05, Wanpeng Li wrote:
> From: Wanpeng Li <wanpeng.li@xxxxxxxxxxx>
>
> 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 occassionally. 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 in 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.
Thanks Wanpend!
However, I'm wondering if we should improve the API instead. Maybe the
token should be passed to irq_bypass_register_{producer,consumer} and
only assigned if the functions succeed. Alex, what do you think?
Thanks,
Paolo
>
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index a29786d..eeaf056 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -415,9 +415,15 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
> irqfd->consumer.stop = kvm_arch_irq_bypass_stop;
> irqfd->consumer.start = kvm_arch_irq_bypass_start;
> ret = irq_bypass_register_consumer(&irqfd->consumer);
> - if (ret)
> + if (ret) {
> pr_info("irq bypass consumer (token %p) registration fails: %d\n",
> irqfd->consumer.token, ret);
> + irqfd->consumer.token = NULL;
> + irqfd->consumer.add_producer = NULL;
> + irqfd->consumer.del_producer = NULL;
> + irqfd->consumer.stop = NULL;
> + irqfd->consumer.start = NULL;
> + }
> }
> #endif
>
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
