From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sheng Yang Subject: Re: [PATCH] intel-iommu: Fix use after release during device attach Date: Tue, 2 Nov 2010 16:00:42 +0800 Message-ID: <201011021600.42802.sheng@linux.intel.com> References: <4CCFB84F.6050102@web.de> <201011021531.22886.sheng@linux.intel.com> Mime-Version: 1.0 Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: Jan Kiszka , Linux Kernel Mailing List , Avi Kivity , Marcelo Tosatti , iommu@lists.linux-foundation.org, David Woodhouse To: kvm@vger.kernel.org Return-path: Received: from mga01.intel.com ([192.55.52.88]:20259 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754465Ab0KBIAg (ORCPT ); Tue, 2 Nov 2010 04:00:36 -0400 In-Reply-To: <201011021531.22886.sheng@linux.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On Tuesday 02 November 2010 15:31:22 Sheng Yang wrote: > On Tuesday 02 November 2010 15:05:51 Jan Kiszka wrote: > > From: Jan Kiszka > > > > Obtail the new pgd pointer before releasing the page containing this > > value. > > > > Signed-off-by: Jan Kiszka > > --- > > > > Who is taking care of this? The kvm tree? > > > > drivers/pci/intel-iommu.c | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c > > index 4789f8e..35463dd 100644 > > --- a/drivers/pci/intel-iommu.c > > +++ b/drivers/pci/intel-iommu.c > > @@ -3627,9 +3627,9 @@ static int intel_iommu_attach_device(struct > > iommu_domain *domain, > > > > pte = dmar_domain->pgd; > > if (dma_pte_present(pte)) { > > > > - free_pgtable_page(dmar_domain->pgd); > > > > dmar_domain->pgd = (struct dma_pte *) > > > > phys_to_virt(dma_pte_addr(pte)); > > > > + free_pgtable_page(pte); > > > > } > > dmar_domain->agaw--; > > > > } > > Reviewed-by: Sheng Yang > > CC iommu mailing list and David. > > OK, Jan, I got your meaning now. And it's not the exactly swap. :) > > I think the old code is safe, seems it's broken(exposed) by: > > commit 1a8bd481bfba30515b54368d90a915db3faf302f > Author: David Woodhouse > Date: Tue Aug 10 01:38:53 2010 +0100 > > intel-iommu: Fix 32-bit build warning with __cmpxchg() In fact this one shouldn't affect the result. Wrong guess... -- regards Yang, Sheng > > drivers/pci/intel-iommu.c: In function 'dma_pte_addr': > drivers/pci/intel-iommu.c:239: warning: passing argument 1 of > '__cmpxchg64' from incompatible pointer typ > > It seems that __cmpxchg64() now cares about the type of its pointer > argument, so give it a (uint64_t *) instead of a pointer to a structure > which contains only that. > > Signed-off-by: David Woodhouse > > diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c > index c9171be..603cdc0 100644 > --- a/drivers/pci/intel-iommu.c > +++ b/drivers/pci/intel-iommu.c > @@ -236,7 +236,7 @@ static inline u64 dma_pte_addr(struct dma_pte *pte) > return pte->val & VTD_PAGE_MASK; > #else > /* Must have a full atomic 64-bit read */ > - return __cmpxchg64(pte, 0ULL, 0ULL) & VTD_PAGE_MASK; > + return __cmpxchg64(&pte->val, 0ULL, 0ULL) & VTD_PAGE_MASK; > #endif > } > > Seems here is the only affected code? > > -- > regards > Yang, Sheng > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html