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

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

Posted by Thorsten Blum 3 weeks, 4 days ago

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 <thorsten.blum@linux.dev>
---
 drivers/iommu/iommufd/selftest.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c
index 7823142097d4..83e2215e7800 100644
--- a/drivers/iommu/iommufd/selftest.c
+++ b/drivers/iommu/iommufd/selftest.c
@@ -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;
 }

Re: [PATCH] iommufd/selftest: Fix page leaks in mock_viommu_{init,destroy}

Posted by Jason Gunthorpe 3 weeks, 3 days ago

On Thu, Mar 12, 2026 at 05:40:42PM +0100, Thorsten Blum wrote:
> 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 <thorsten.blum@linux.dev>
> ---
>  drivers/iommu/iommufd/selftest.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied, thanks

Jason

Re: [PATCH] iommufd/selftest: Fix page leaks in mock_viommu_{init,destroy}

Posted by Nicolin Chen 3 weeks, 4 days ago

On Thu, Mar 12, 2026 at 05:40:42PM +0100, Thorsten Blum wrote:
> 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 <thorsten.blum@linux.dev>

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

Re: [PATCH] iommufd/selftest: Fix page leaks in mock_viommu_{init,destroy}

Posted by Pranjal Shrivastava 3 weeks, 4 days ago

On Thu, Mar 12, 2026 at 05:40:42PM +0100, Thorsten Blum wrote:
> 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 <thorsten.blum@linux.dev>

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 <praan@google.com>

> ---
>  drivers/iommu/iommufd/selftest.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c
> index 7823142097d4..83e2215e7800 100644
> --- a/drivers/iommu/iommufd/selftest.c
> +++ b/drivers/iommu/iommufd/selftest.c
> @@ -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;
>  }
>