From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sheng Yang Subject: [PATCH 15/15] KVM: Fix racy in kvm_free_assigned_irq Date: Thu, 25 Dec 2008 17:09:39 +0800 Message-ID: <1230196179-6918-16-git-send-email-sheng@linux.intel.com> References: <1230196179-6918-1-git-send-email-sheng@linux.intel.com> Cc: kvm@vger.kernel.org, Sheng Yang To: Avi Kivity , Marcelo Tosatti Return-path: Received: from mga14.intel.com ([143.182.124.37]:26810 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751682AbYLYJJr (ORCPT ); Thu, 25 Dec 2008 04:09:47 -0500 In-Reply-To: <1230196179-6918-1-git-send-email-sheng@linux.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: Thanks to Marcelo's observation, The following code have potential issue: if (cancel_work_sync(&assigned_dev->interrupt_work)) kvm_put_kvm(kvm); In fact, cancel_work_sync() would return true either work struct is only scheduled or the callback of work struct is executed. This code only consider the former situation. Also, we have a window between cancel_work_sync() and free_irq. This patch fixs them two. Signed-off-by: Sheng Yang --- virt/kvm/kvm_main.c | 26 ++++++++++++++++++++++---- 1 files changed, 22 insertions(+), 4 deletions(-) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 1863942..ed10f15 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -186,10 +186,28 @@ static void kvm_free_assigned_irq(struct kvm *kvm, if (!assigned_dev->irq_requested_type) return; - if (cancel_work_sync(&assigned_dev->interrupt_work)) - /* We had pending work. That means we will have to take - * care of kvm_put_kvm. - */ + /* + * We need to ensure: kvm_put_kvm() paired with kvm_get_kvm() in + * kvm_assigned_dev_intr, and no more interrupt after we cancelled + * current one. + * + * Here we have two possiblities for cancel_work_sync() return true: + * 1. The work is scheduled, but callback haven't been called. We need + * to call kvm_put_kvm() here. And IRQ is already disabled without + * doubt. + * + * 2. The callback have executed, here we don't need to call + * kvm_put_kvm(), but we may need to disable irq(e.g. for MSI). + * + * We judge the two condition according to if we have pending IRQs in + * irq_fifo. + */ + if (kfifo_len(assigned_dev->irq_fifo) == 0 && + (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI)) + disable_irq_nosync(assigned_dev->host_irq); + + if (cancel_work_sync(&assigned_dev->interrupt_work) && + kfifo_len(assigned_dev->irq_fifo) != 0) kvm_put_kvm(kvm); free_irq(assigned_dev->host_irq, (void *)assigned_dev); -- 1.5.4.5