[PATCH] iommu/vt-d: Fix infinite loop in free_all_cpu_cached_iovas

[PATCH] iommu/vt-d: Fix infinite loop in free_all_cpu_cached_iovas

[Aaron Campbell]

Per VT-d spec Section 10.4.2 ("Capability Register"), the maximum
number of possible domains is 64K; indeed this is the maximum value
that the cap_ndoms() macro will expand to.  Since the value 65536
will not fix in a u16, the 'did' variable must be promoted to an
int, otherwise the test for < 65536 will always be true and the
loop will never end.

The symptom, in my case, was a hung machine during suspend.

Fixes: 3bd4f9112f87 ("iommu/vt-d: Fix overflow of iommu->domains array")
Signed-off-by: Aaron Campbell <aaron-J6AcJDG0ZEwdnm+yROfE0A@public.gmane.org>
---
 drivers/iommu/intel-iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index cfe410e..323dac9 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4602,13 +4602,13 @@ static void free_all_cpu_cached_iovas(unsigned int cpu)
 	for (i = 0; i < g_num_of_iommus; i++) {
 		struct intel_iommu *iommu = g_iommus[i];
 		struct dmar_domain *domain;
-		u16 did;
+		int did;
 
 		if (!iommu)
 			continue;
 
 		for (did = 0; did < cap_ndoms(iommu->cap); did++) {
-			domain = get_iommu_domain(iommu, did);
+			domain = get_iommu_domain(iommu, (u16)did);
 
 			if (!domain)
 				continue;
-- 
2.7.4

Re: [PATCH] iommu/vt-d: Fix infinite loop in free_all_cpu_cached_iovas

[Jan Niehusmann]

On Sat, Jul 02, 2016 at 09:23:24PM -0300, Aaron Campbell wrote:
> Per VT-d spec Section 10.4.2 ("Capability Register"), the maximum
> number of possible domains is 64K; indeed this is the maximum value
> that the cap_ndoms() macro will expand to.

Good catch. Before 3bd4f9112f87, the loop ended at the fixed value
0xffff. I didn't notice that cap_ndoms() could not only be lower than
that, like on my notebook, but could also be greater. Therefore I didn't
even look at the type of 'did'.

So, strictly speaking, the original code would not only overflow for
small values of cap_ndoms, but would miss the entry at 0xffff in case
cap_ndoms was 0x10000. My patch unfortunately changed that to an endless
loop, instead.

Jan

Re: [PATCH] iommu/vt-d: Fix infinite loop in free_all_cpu_cached_iovas

[Joerg Roedel]

On Sat, Jul 02, 2016 at 09:23:24PM -0300, Aaron Campbell wrote:
> Per VT-d spec Section 10.4.2 ("Capability Register"), the maximum
> number of possible domains is 64K; indeed this is the maximum value
> that the cap_ndoms() macro will expand to.  Since the value 65536
> will not fix in a u16, the 'did' variable must be promoted to an
> int, otherwise the test for < 65536 will always be true and the
> loop will never end.
> 
> The symptom, in my case, was a hung machine during suspend.

Applied, thanks.