Re: [PATCH] amd_iommu: Fix leak in free_pagetable()

[Joerg Roedel]

On Mon, Jun 17, 2013 at 07:52:14PM -0600, Alex Williamson wrote:
>  static void free_pagetable(struct protection_domain *domain)
>  {
> -	int i, j;
> -	u64 *p1, *p2, *p3;
> +	int i, j, k, l, m, depth = domain->mode;
> +	u64 *p1, *p2, *p3, *p4, *p5, *p6;
>  
>  	p1 = domain->pt_root;
>  
>  	if (!p1)
>  		return;
>  
> -	for (i = 0; i < 512; ++i) {
> +	for (i = 0; depth > 1 && i < 512; ++i) {
>  		if (!IOMMU_PTE_PRESENT(p1[i]))
>  			continue;
>  
>  		p2 = IOMMU_PTE_PAGE(p1[i]);
> -		for (j = 0; j < 512; ++j) {
> +		for (j = 0; depth > 2 && j < 512; ++j) {
>  			if (!IOMMU_PTE_PRESENT(p2[j]))
>  				continue;
> +
>  			p3 = IOMMU_PTE_PAGE(p2[j]);
> +			for (k = 0; depth > 3 && k < 512; ++k) {
> +				if (!IOMMU_PTE_PRESENT(p3[k]))
> +					continue;
> +
> +				p4 = IOMMU_PTE_PAGE(p3[k]);
> +				for (l = 0; depth > 4 && l < 512; ++l) {
> +					if (!IOMMU_PTE_PRESENT(p4[l]))
> +						continue;
> +
> +					p5 = IOMMU_PTE_PAGE(p4[l]);
> +					for (m = 0; depth > 5 && m < 512; ++m) {
> +						if (!IOMMU_PTE_PRESENT(p5[m]))
> +							continue;
> +						p6 = IOMMU_PTE_PAGE(p5[m]);
> +						free_page((unsigned long)p6);
> +					}
> +
> +					free_page((unsigned long)p5);
> +				}
> +
> +				free_page((unsigned long)p4);
> +			}
> +
>  			free_page((unsigned long)p3);
>  		}

Hmm, actually a recursive version would make more sense here. But since
recursion is a bad idea in the kernel, how about this approach instead: