Skip to contentSkip to conversation

iommufd/selftest: Fix page leaks in mock_viommu_{init,destroy}

PATCH Applied Thorsten Blum 4 participants 4 messages Mar 12 – Mar 13

Subsystems (2)
IOMMU Iommufd
lore
Share & tools
lore

Thorsten Blum

author
#0 lore raw

PATCH 1/1 iommufd/selftest: Fix page leaks in mock_viommu_{init,destroy}

mock_viommu_init() allocates two pages using __get_free_pages(..., 1), but its error path and mock_viommu_destroy() only release the first page using free_page(), leaking the second page. Use free_pages() with the matching order instead to avoid any page leaks.

Fixes 80478a2b450e ("iommufd/selftest: Add coverage for the new mmap interface")
Signed-off-by Thorsten Blum <[email protected]>
---
 drivers/iommu/iommufd/selftest.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iommu/iommufd/selftest.c +2 -2
index 7823142097d4..83e2215e7800 100644
@@ -636,7 +636,7 @@ static void mock_viommu_destroy(struct iommufd_viommu *viommu)
 	if (mock_viommu->mmap_offset)
 		iommufd_viommu_destroy_mmap(&mock_viommu->core,
 					    mock_viommu->mmap_offset);
-	free_page((unsigned long)mock_viommu->page);
+	free_pages((unsigned long)mock_viommu->page, 1);
 	mutex_destroy(&mock_viommu->queue_mutex);
 
 	/* iommufd core frees mock_viommu and viommu */

@@ -870,7 +870,7 @@ static int mock_viommu_init(struct iommufd_viommu *viommu,
 	iommufd_viommu_destroy_mmap(&mock_viommu->core,
 				    mock_viommu->mmap_offset);
 err_free_page:
-	free_page((unsigned long)mock_viommu->page);
+	free_pages((unsigned long)mock_viommu->page, 1);
 	return rc;
 }
Reviewed-by on v1 1/1
#1 lore raw
Quoted 7 lines · Thorsten Blum

mock_viommu_init() allocates two pages using __get_free_pages(..., 1), but its error path and mock_viommu_destroy() only release the first page using free_page(), leaking the second page. Use free_pages() with the matching order instead to avoid any page leaks.

Fixes 80478a2b450e ("iommufd/selftest: Add coverage for the new mmap interface")
Signed-off-by Thorsten Blum <[email protected]>

Thanks for the fix. We indeed allocate two pages but free only the first one. I guess we missed that earlier.

Reviewed-by Pranjal Shrivastava <[email protected]>
Quoted 26 lines
---
 drivers/iommu/iommufd/selftest.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iommu/iommufd/selftest.c +2 -2
index 7823142097d4..83e2215e7800 100644
@@ -636,7 +636,7 @@ static void mock_viommu_destroy(struct iommufd_viommu *viommu)
 	if (mock_viommu->mmap_offset)
 		iommufd_viommu_destroy_mmap(&mock_viommu->core,
 					    mock_viommu->mmap_offset);
-	free_page((unsigned long)mock_viommu->page);
+	free_pages((unsigned long)mock_viommu->page, 1);
 	mutex_destroy(&mock_viommu->queue_mutex);
 
 	/* iommufd core frees mock_viommu and viommu */

@@ -870,7 +870,7 @@ static int mock_viommu_init(struct iommufd_viommu *viommu,
 	iommufd_viommu_destroy_mmap(&mock_viommu->core,
 				    mock_viommu->mmap_offset);
 err_free_page:
-	free_page((unsigned long)mock_viommu->page);
+	free_pages((unsigned long)mock_viommu->page, 1);
 	return rc;
 }
Reviewed-by on v1 1/1
#2 lore raw
In reply to Thorsten Blum
Quoted 7 lines · Thorsten Blum

mock_viommu_init() allocates two pages using __get_free_pages(..., 1), but its error path and mock_viommu_destroy() only release the first page using free_page(), leaking the second page. Use free_pages() with the matching order instead to avoid any page leaks.

Fixes 80478a2b450e ("iommufd/selftest: Add coverage for the new mmap interface")
Signed-off-by Thorsten Blum <[email protected]>
Reviewed-by Nicolin Chen <[email protected]>
Applied
#3 lore raw
In reply to Thorsten Blum
Quoted 10 lines · Thorsten Blum

mock_viommu_init() allocates two pages using __get_free_pages(..., 1), but its error path and mock_viommu_destroy() only release the first page using free_page(), leaking the second page. Use free_pages() with the matching order instead to avoid any page leaks.

Fixes 80478a2b450e ("iommufd/selftest: Add coverage for the new mmap interface")
Signed-off-by Thorsten Blum <[email protected]>
---
 drivers/iommu/iommufd/selftest.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Applied, thanks

Jason