linux-kernel - [PATCH][RFC] Intel IOMMU: get_domain_for_dev() leaks a bit of mem if dmar_find_matched_drhd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LNX.2.00.1102062102390.13593@swampdragon.chaosbits.net>
Date:	Sun, 6 Feb 2011 21:11:33 +0100 (CET)
From:	Jesper Juhl <jj@...osbits.net>
To:	linux-kernel@...r.kernel.org
cc:	linux-pci@...r.kernel.org, iommu@...ts.linux-foundation.org,
	Jesse Barnes <jbarnes@...tuousgeek.org>,
	David Woodhouse <dwmw2@...radead.org>,
	Ashok Raj <ashok.raj@...el.com>,
	Shaohua Li <shaohua.li@...el.com>,
	Anil S Keshavamurthy <anil.s.keshavamurthy@...el.com>,
	Fenghua Yu <fenghua.yu@...el.com>
Subject: [PATCH][RFC] Intel IOMMU: get_domain_for_dev() leaks a bit of mem
 if dmar_find_matched_drhd_unit() fails.

I believe that there's a small memory leak in 
drivers/pci/intel-iommu.c:get_domain_for_dev().

If the call to alloc_domain() succeeds but the subsequent call to 
dmar_find_matched_drhd_unit() fails, then the current code will return 
NULL without calling domain_exit(domain) which will leak the memory that 
alloc_domain() allocated.

The easy fix for that is to simply move the call to alloc_domain() below 
the call to dmar_find_matched_drhd_unit() since the latter does not depend 
on the former.

I also made the change of moving the assignment to local variable 'iommu' 
below both calls since there is no point in doing that work if either of 
those those calls fail.

I also changed the 'return NULL' in the dmar_find_matched_drhd_unit() 
failure case to a 'goto error' since I figured that if rechecking 
'find_domain(pdev)' makes sense after a alloc_domain() failure then it 
would also make sense after a dmar_find_matched_drhd_unit() failure.

Patch is only compile tested and I'm not very familliar with this code at 
all, so please review carefully before applying and please feel free to 
provide feedback if this patch is somehow not making sense.

Signed-off-by: Jesper Juhl <jj@...osbits.net>
---
 intel-iommu.c |   11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index 4789f8e..dfbdb08 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -1820,19 +1820,18 @@ static struct dmar_domain *get_domain_for_dev(struct pci_dev *pdev, int gaw)
 		}
 	}

-	domain = alloc_domain();
-	if (!domain)
-		goto error;
-
 	/* Allocate new domain for the device */
 	drhd = dmar_find_matched_drhd_unit(pdev);
 	if (!drhd) {
 		printk(KERN_ERR "IOMMU: can't find DMAR for device %s\n",
 			pci_name(pdev));
-		return NULL;
+		goto error;
 	}
-	iommu = drhd->iommu;
+	domain = alloc_domain();
+	if (!domain)
+		goto error;

+	iommu = drhd->iommu;
 	ret = iommu_attach_domain(domain, iommu);
 	if (ret) {
 		domain_exit(domain);

-- 
Jesper Juhl <jj@...osbits.net>            http://www.chaosbits.net/
Plain text mails only, please.
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/