linux-kernel - Re: [PATCH] KVM: eventfd: fix NULL deref irqbypass consumer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 5 Jan 2017 18:51:32 +0800
From:   Wanpeng Li <kernellwp@...il.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        kvm <kvm@...r.kernel.org>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Wanpeng Li <wanpeng.li@...mail.com>,
        Alex Williamson <alex.williamson@...hat.com>
Subject: Re: [PATCH] KVM: eventfd: fix NULL deref irqbypass consumer

2017-01-05 18:24 GMT+08:00 Paolo Bonzini <pbonzini@...hat.com>:
>
>
> On 05/01/2017 10:05, Wanpeng Li wrote:
>> From: Wanpeng Li <wanpeng.li@...mail.com>
>>
>> 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!

NP. :)

>
> 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?

Yeah, I can continue to improve the patch if we reach an agreement with Alex. :)

Regards,
Wanpeng Li