A comment warning against this bug is there, but the code is not doing what the comment says. Therefore it is possible that an EPOLLHUP races against irq_bypass_register_consumer. The EPOLLHUP handler schedules irqfd_shutdown, and if that runs soon enough, you get a use-after-free.
Reported-by: syzbot syzkaller@googlegroups.com Cc: stable@vger.kernel.org Signed-off-by: Paolo Bonzini pbonzini@redhat.com --- virt/kvm/eventfd.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c index 6e865e8b5b10..44dda5dad0ee 100644 --- a/virt/kvm/eventfd.c +++ b/virt/kvm/eventfd.c @@ -402,11 +402,6 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args) if (events & EPOLLIN) schedule_work(&irqfd->inject);
- /* - * do not drop the file until the irqfd is fully initialized, otherwise - * we might race against the EPOLLHUP - */ - fdput(f); #ifdef CONFIG_HAVE_KVM_IRQ_BYPASS if (kvm_arch_has_irq_bypass()) { irqfd->consumer.token = (void *)irqfd->eventfd; @@ -421,6 +416,11 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args) } #endif
+ /* + * do not drop the file until the irqfd is fully initialized, otherwise + * we might race against the EPOLLHUP + */ + fdput(f); return 0;
fail:
On 28.05.2018 13:31, Paolo Bonzini wrote:
A comment warning against this bug is there, but the code is not doing what the comment says. Therefore it is possible that an EPOLLHUP races against irq_bypass_register_consumer. The EPOLLHUP handler schedules irqfd_shutdown, and if that runs soon enough, you get a use-after-free.
Reported-by: syzbot syzkaller@googlegroups.com Cc: stable@vger.kernel.org Signed-off-by: Paolo Bonzini pbonzini@redhat.com
virt/kvm/eventfd.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c index 6e865e8b5b10..44dda5dad0ee 100644 --- a/virt/kvm/eventfd.c +++ b/virt/kvm/eventfd.c @@ -402,11 +402,6 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args) if (events & EPOLLIN) schedule_work(&irqfd->inject);
- /*
* do not drop the file until the irqfd is fully initialized, otherwise* we might race against the EPOLLHUP*/- fdput(f);
#ifdef CONFIG_HAVE_KVM_IRQ_BYPASS if (kvm_arch_has_irq_bypass()) { irqfd->consumer.token = (void *)irqfd->eventfd; @@ -421,6 +416,11 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args) } #endif
- /*
* do not drop the file until the irqfd is fully initialized, otherwise* we might race against the EPOLLHUP*/- fdput(f); return 0;
fail:
Reviewed-by: David Hildenbrand david@redhat.com
linux-stable-mirror@lists.linaro.org
-
David Hildenbrand -
Paolo Bonzini