From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joerg Roedel Subject: Re: [PATCH] iommu/vt-d: Fix VM domain ID leak Date: Thu, 16 Jul 2015 17:20:30 +0200 Message-ID: <20150716152030.GB10969@8bytes.org> References: <20150714204731.10189.28556.stgit@gimli.home> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20150714204731.10189.28556.stgit-GCcqpEzw8uZBDLzU/O5InQ@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Alex Williamson Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org, jiang.liu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: iommu@lists.linux-foundation.org On Tue, Jul 14, 2015 at 02:48:53PM -0600, Alex Williamson wrote: > This continues the attempt to fix commit fb170fb4c548 ("iommu/vt-d: > Introduce helper functions to make code symmetric for readability"). > The previous attempt in commit 71684406905f ("iommu/vt-d: Detach > domain *only* from attached iommus") overlooked the fact that > dmar_domain.iommu_bmp gets cleared for VM domains when devices are > detached: > > intel_iommu_detach_device > domain_remove_one_dev_info > domain_detach_iommu > > The domain is detached from the iommu, but the iommu is still attached > to the domain, for whatever reason. Gaah, this whole vm_or_si special handling is a complete and fragile mess. The only reason for keeping the domain-id allocated in the iommu seems to be that it can be re-used later, when another device behind this iommu is attached to the domain. But this is hardly a justification for the complexity and special case handling here, so how about this diff instead: diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index a98a7b2..c8fc8c8 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -4618,8 +4618,7 @@ static void domain_remove_one_dev_info(struct dmar_domain *domain, if (found == 0) { domain_detach_iommu(domain, iommu); - if (!domain_type_is_vm_or_si(domain)) - iommu_detach_domain(domain, iommu); + iommu_detach_domain(domain, iommu); } } This removes the caching of domain-ids, and iommu_detach_domain correctly handles all types of domains (dma-api, vm and si). It should be safe, but can you please double-check? Joerg