From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753748Ab0KBIAn (ORCPT ); Tue, 2 Nov 2010 04:00:43 -0400 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 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.58,279,1286175600"; d="scan'208";a="853259410" From: Sheng Yang Organization: Intel Opensource Technology Center To: kvm@vger.kernel.org Subject: Re: [PATCH] intel-iommu: Fix use after release during device attach Date: Tue, 2 Nov 2010 16:00:42 +0800 User-Agent: KMail/1.13.5 (Linux/2.6.35-22-generic; KDE/4.5.1; x86_64; ; ) Cc: Jan Kiszka , Linux Kernel Mailing List , Avi Kivity , Marcelo Tosatti , iommu@lists.linux-foundation.org, David Woodhouse References: <4CCFB84F.6050102@web.de> <201011021531.22886.sheng@linux.intel.com> In-Reply-To: <201011021531.22886.sheng@linux.intel.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Message-Id: <201011021600.42802.sheng@linux.intel.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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