bug in intel_iommu_unmap()

["Shankar, Hari" <Hari.Shankar-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 591 bytes --]

Hi David,
NetApp is using Linux VFIO code for user space drivers. We recently ran into a memory corruption bug which was root caused to lack of IOMMU TLB flush in intel_iommu_unmap() routine.

While reviewing the code we also figured that the unmap routine always returns size only for one page rather than the total unmapped size. Since VFIO unmaps one page at a time, the problem isn't exposed

Alex Williamson suggested that you're the maintainer of the code so sending to you for review.

Diff for the changes is attached and is generated on Linux kernel version 3.6.11

Hari.

[-- Attachment #1.2: Type: text/html, Size: 1043 bytes --]

[-- Attachment #2: intel_iommu_unmap_patch.txt --]
[-- Type: text/plain, Size: 1695 bytes --]

--- drivers/iommu/intel-iommu.c.orig    2013-09-01 10:10:14.723958000 -0700
+++ drivers/iommu/intel-iommu.c 2013-09-01 10:17:22.428412000 -0700
@@ -4060,14 +4060,34 @@ static size_t intel_iommu_unmap(struct i
 {
        struct dmar_domain *dmar_domain = domain->priv;
        int order;
+       struct intel_iommu *iommu;
+       unsigned long start_pfn, last_pfn;
+       unsigned int npages;
+       int iommu_id, num, ndomains;

-       order = dma_pte_clear_range(dmar_domain, iova >> VTD_PAGE_SHIFT,
-                           (iova + size - 1) >> VTD_PAGE_SHIFT);
+       start_pfn = iova >> VTD_PAGE_SHIFT;
+       last_pfn = (iova + size - 1) >> VTD_PAGE_SHIFT;
+       order = dma_pte_clear_range(dmar_domain, start_pfn, last_pfn);

-       if (dmar_domain->max_addr == iova + size)
-               dmar_domain->max_addr = iova;
+       last_pfn |= (1UL << order) - 1;
+       npages = last_pfn - start_pfn + 1;
+
+       for_each_set_bit(iommu_id, dmar_domain->iommu_bmp, g_num_of_iommus) {
+               iommu = g_iommus[iommu_id];

-       return PAGE_SIZE << order;
+               /*
+                * find bit position of dmar_domain
+                */
+               ndomains = cap_ndoms(iommu->cap);
+               for_each_set_bit(num, iommu->domain_ids, ndomains)
+                       if (iommu->domains[num] == dmar_domain)
+                               iommu_flush_iotlb_psi(iommu, num, start_pfn, npages, 0);
+       }
+
+       if (dmar_domain->max_addr <= iova + (npages << VTD_PAGE_SHIFT))
+               dmar_domain->max_addr = iova;
+
+       return npages << VTD_PAGE_SHIFT;
 }

 static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain,


[-- Attachment #3: Type: text/plain, Size: 0 bytes --]