iommu/dma: Fix MSI reservation allocation
Subsystems (3)
PATCH v2 1/1 iommu/dma: Fix MSI reservation allocation
The way cookie_init_hw_msi_region() allocates the iommu_dma_msi_page structures doesn't match the way iommu_put_dma_cookie() frees them.
The former performs a single allocation of all the required structures, while the latter tries to free them one at a time. It doesn't quite work for the main use case (the GICv3 ITS where the range is 64kB) when the base granule size is 4kB.
This leads to a nice slab corruption on teardown, which is easily observable by simply creating a VF on a SRIOV-capable device, and tearing it down immediately (no need to even make use of it). Fortunately, this only affects systems where the ITS isn't translated by the SMMU, which are both rare and non-standard.
Fix it by allocating iommu_dma_msi_page structures one at a time.
---
* From v1:
- Got rid of the superfluous error handling (Robin)
- Clarified that it only affects a very small set of systems
- Added Eric's RB (which I assume still stands)
drivers/iommu/dma-iommu.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/iommu/dma-iommu.c +8 -8
@@ -177,15 +177,15 @@ static int cookie_init_hw_msi_region(struct iommu_dma_cookie *cookie, start -=iova_offset (iovad, start); num_pages =iova_align (iovad, end - start) >>iova_shift (iovad); - msi_page =kcalloc (num_pages,sizeof (*msi_page),GFP_KERNEL ); -if (!msi_page) -return -ENOMEM ; -for (i =0 ; i < num_pages; i++) { - msi_page[i].phys = start; - msi_page[i].iova = start; -INIT_LIST_HEAD (&msi_page[i].list); -list_add (&msi_page[i].list, &cookie->msi_page_list); + msi_page =kmalloc (sizeof (*msi_page),GFP_KERNEL ); +if (!msi_page) +return -ENOMEM ; + + msi_page->phys = start; + msi_page->iova = start; +INIT_LIST_HEAD (&msi_page->list); +list_add (&msi_page->list, &cookie->msi_page_list); start += iovad->granule; }
Signature · 1 line
2.20.1
Quoted 8 lines · Marc Zyngier
The way
cookie_init_hw_msi_region()allocates theiommu_dma_msi_pagestructures doesn't match the wayiommu_put_dma_cookie()frees them.The former performs a single allocation of all the required structures, while the latter tries to free them one at a time. It doesn't quite work for the main use case (the GICv3 ITS where the range is 64kB) when the base granule size is 4kB.
Quoted 7 lines
This leads to a nice slab corruption on teardown, which is easily observable by simply creating a VF on a SRIOV-capable device, and tearing it down immediately (no need to even make use of it). Fortunately, this only affects systems where the ITS isn't translated by the SMMU, which are both rare and non-standard.
Fix it by allocating
iommu_dma_msi_pagestructures one at a time.
Quoted 45 lines
Fixes 7c1b058c8b5a3 ("iommu/dma: Handle IOMMU API reserved regions")Signed-off-by Marc Zyngier <[email protected]>Reviewed-by Eric Auger <[email protected]>Cc Robin Murphy <[email protected]>Cc Joerg Roedel <[email protected]>Cc Will Deacon <[email protected]>---
* From v1:
- Got rid of the superfluous error handling (Robin)
- Clarified that it only affects a very small set of systems
- Added Eric's RB (which I assume still stands)drivers/iommu/dma-iommu.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)drivers/iommu/dma-iommu.c +8
-8@@ -177,15 +177,15 @@ static int cookie_init_hw_msi_region(struct iommu_dma_cookie *cookie, start -=iova_offset (iovad, start); num_pages =iova_align (iovad, end - start) >>iova_shift (iovad); - msi_page =kcalloc (num_pages,sizeof (*msi_page),GFP_KERNEL ); -if (!msi_page) -return -ENOMEM ; -for (i =0 ; i < num_pages; i++) { - msi_page[i].phys = start; - msi_page[i].iova = start; -INIT_LIST_HEAD (&msi_page[i].list); -list_add (&msi_page[i].list, &cookie->msi_page_list); + msi_page =kmalloc (sizeof (*msi_page),GFP_KERNEL ); +if (!msi_page) +return -ENOMEM ; + + msi_page->phys = start; + msi_page->iova = start; +INIT_LIST_HEAD (&msi_page->list); +list_add (&msi_page->list, &cookie->msi_page_list); start += iovad->granule; }
Quoted 16 lines · Marc Zyngier
The way
cookie_init_hw_msi_region()allocates theiommu_dma_msi_pagestructures doesn't match the wayiommu_put_dma_cookie()frees them.The former performs a single allocation of all the required structures, while the latter tries to free them one at a time. It doesn't quite work for the main use case (the GICv3 ITS where the range is 64kB) when the base granule size is 4kB.
This leads to a nice slab corruption on teardown, which is easily observable by simply creating a VF on a SRIOV-capable device, and tearing it down immediately (no need to even make use of it). Fortunately, this only affects systems where the ITS isn't translated by the SMMU, which are both rare and non-standard.
Fix it by allocating
iommu_dma_msi_pagestructures one at a time.
Quoted 7 lines
Fixes 7c1b058c8b5a3 ("iommu/dma: Handle IOMMU API reserved regions")Signed-off-by Marc Zyngier <[email protected]>Reviewed-by Eric Auger <[email protected]>Cc Robin Murphy <[email protected]>Cc Joerg Roedel <[email protected]>Cc Will Deacon <[email protected]>
Applied for v5.6, thanks.