Skip to contentSkip to conversation

media: omap3isp: fix unbalanced dma_iommu_mapping

PATCH Superseded Suman Anna 4 participants 5 messages Mar 14, 2018 – Apr 11, 2018 Media (V4L/DVB)

lore
Share & tools
lore

Suman Anna

author
#0 lore raw

PATCH v2 1/1 media: omap3isp: fix unbalanced dma_iommu_mapping

The OMAP3 ISP driver manages its MMU mappings through the IOMMU-aware ARM DMA backend. The current code creates a dma_iommu_mapping and attaches this to the ISP device, but never detaches the mapping in either the probe failure paths or the driver remove path resulting in an unbalanced mapping refcount and a memory leak. Fix this properly.

Reported-by Pavel Machek <[email protected]>
Signed-off-by Suman Anna <[email protected]>
Acked-by Sakari Ailus <[email protected]>

---
v2 Changes:
- Dropped the error_attach label, and returned directly from the first error path (comments from Sakari)
- Added Sakari's Acked-by
v1: https://patchwork.kernel.org/patch/10276759/

Pavel,
I dropped your Tested-by from v2 since I modified the patch, can you recheck the new patch again? Thanks.

regards
Suman

 drivers/media/platform/omap3isp/isp.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/media/platform/omap3isp/isp.c +4 -3
index 8eb000e3d8fd..f2db5128d786 100644
@@ -1945,6 +1945,7 @@ static int isp_initialize_modules(struct isp_device *isp)
 
 static void isp_detach_iommu(struct isp_device *isp)
 {
+	arm_iommu_detach_device(isp->dev);
 	arm_iommu_release_mapping(isp->mapping);
 	isp->mapping = NULL;
 }

@@ -1961,8 +1962,7 @@ static int isp_attach_iommu(struct isp_device *isp)
 	mapping = arm_iommu_create_mapping(&platform_bus_type, SZ_1G, SZ_2G);
 	if (IS_ERR(mapping)) {
 		dev_err(isp->dev, "failed to create ARM IOMMU mapping\n");
-		ret = PTR_ERR(mapping);
-		goto error;
+		return PTR_ERR(mapping);
 	}
 
 	isp->mapping = mapping;

@@ -1977,7 +1977,8 @@ static int isp_attach_iommu(struct isp_device *isp)
 	return 0;
 
 error:
-	isp_detach_iommu(isp);
+	arm_iommu_release_mapping(isp->mapping);
+	isp->mapping = NULL;
 	return ret;
 }
Signature · 1 line
2.16.2
#1 lore raw

PATCH v2 1/1 media: omap3isp: fix unbalanced dma_iommu_mapping

Quoted 16 lines · Suman Anna

The OMAP3 ISP driver manages its MMU mappings through the IOMMU-aware ARM DMA backend. The current code creates a dma_iommu_mapping and attaches this to the ISP device, but never detaches the mapping in either the probe failure paths or the driver remove path resulting in an unbalanced mapping refcount and a memory leak. Fix this properly.

Reported-by Pavel Machek <[email protected]>
Signed-off-by Suman Anna <[email protected]>
Acked-by Sakari Ailus <[email protected]>

---
v2 Changes:
- Dropped the error_attach label, and returned directly from the first error path (comments from Sakari)
- Added Sakari's Acked-by
v1: https://patchwork.kernel.org/patch/10276759/

Quoted 3 lines

Pavel,
I dropped your Tested-by from v2 since I modified the patch, can you recheck the new patch again? Thanks.

I updated to new -next version and re-ran the test.

Tested-by Pavel Machek <[email protected]>
Signature · 9 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180315/f6f5b43e/attachment.sig>
#2 lore raw

PATCH v2 1/1 media: omap3isp: fix unbalanced dma_iommu_mapping

In reply to Suman Anna

Hi Suman,

Thank you for the patch.

Quoted 9 lines · On Wednesday, 14 March 2018 17:41:36 EET Suman Anna wrote:

The OMAP3 ISP driver manages its MMU mappings through the IOMMU-aware ARM DMA backend. The current code creates a dma_iommu_mapping and attaches this to the ISP device, but never detaches the mapping in either the probe failure paths or the driver remove path resulting in an unbalanced mapping refcount and a memory leak. Fix this properly.

Reported-by Pavel Machek <[email protected]>
Signed-off-by Suman Anna <[email protected]>
Acked-by Sakari Ailus <[email protected]>
Reviewed-by Laurent Pinchart <[email protected]>
Quoted 50 lines

---
v2 Changes:
- Dropped the error_attach label, and returned directly from the first error path (comments from Sakari)
- Added Sakari's Acked-by
v1: https://patchwork.kernel.org/patch/10276759/

Pavel,
I dropped your Tested-by from v2 since I modified the patch, can you recheck the new patch again? Thanks.

regards
Suman

 drivers/media/platform/omap3isp/isp.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
(quoted diff) +0 -0

b/drivers/media/platform/omap3isp/isp.c index 8eb000e3d8fd..f2db5128d786 100644

drivers/media/platform/omap3isp/isp.c +1 -0
@@ -1945,6 +1945,7 @@ static int isp_initialize_modules(struct isp_device
 *isp)
 
 static void isp_detach_iommu(struct isp_device *isp)
 {
+	arm_iommu_detach_device(isp->dev);
 	arm_iommu_release_mapping(isp->mapping);
 	isp->mapping = NULL;

}

drivers/media/platform/omap3isp/isp.c +3 -3
@@ -1961,8 +1962,7 @@ static int isp_attach_iommu(struct isp_device *isp)
 	mapping = arm_iommu_create_mapping(&platform_bus_type, SZ_1G, SZ_2G);
 	if (IS_ERR(mapping)) {
 		dev_err(isp->dev, "failed to create ARM IOMMU mapping\n");
-		ret = PTR_ERR(mapping);
-		goto error;
+		return PTR_ERR(mapping);
 	}
 
 	isp->mapping = mapping;

@@ -1977,7 +1977,8 @@ static int isp_attach_iommu(struct isp_device *isp)
 	return 0;
 
 error:
-	isp_detach_iommu(isp);
+	arm_iommu_release_mapping(isp->mapping);
+	isp->mapping = NULL;
 	return ret;
 }
Signature · 3 lines
Regards,

Laurent Pinchart
author
#3 lore raw

PATCH v2 1/1 media: omap3isp: fix unbalanced dma_iommu_mapping

Hi Mauro,

Quoted 11 lines · Laurent Pinchart

Hi Suman,

Thank you for the patch.

Quoted 6 lines · On Wednesday, 14 March 2018 17:41:36 EET Suman Anna wrote:

The OMAP3 ISP driver manages its MMU mappings through the IOMMU-aware ARM DMA backend. The current code creates a dma_iommu_mapping and attaches this to the ISP device, but never detaches the mapping in either the probe failure paths or the driver remove path resulting in an unbalanced mapping refcount and a memory leak. Fix this properly.

Quoted 5 lines
Quoted 3 lines
Reported-by Pavel Machek <[email protected]>
Signed-off-by Suman Anna <[email protected]>
Acked-by Sakari Ailus <[email protected]>
Reviewed-by Laurent Pinchart <[email protected]>

I don't see this patch in -next yet, can you pick up this patch at your earliest?

Thanks,
Suman

Quoted 52 lines
Quoted 51 lines

---
v2 Changes:
- Dropped the error_attach label, and returned directly from the first error path (comments from Sakari)
- Added Sakari's Acked-by
v1: https://patchwork.kernel.org/patch/10276759/

Pavel,
I dropped your Tested-by from v2 since I modified the patch, can you recheck the new patch again? Thanks.

regards
Suman

 drivers/media/platform/omap3isp/isp.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
(quoted diff) +0 -0

b/drivers/media/platform/omap3isp/isp.c index 8eb000e3d8fd..f2db5128d786 100644

drivers/media/platform/omap3isp/isp.c +1 -0
@@ -1945,6 +1945,7 @@ static int isp_initialize_modules(struct isp_device
 *isp)
 
 static void isp_detach_iommu(struct isp_device *isp)
 {
+	arm_iommu_detach_device(isp->dev);
 	arm_iommu_release_mapping(isp->mapping);
 	isp->mapping = NULL;

}

drivers/media/platform/omap3isp/isp.c +3 -3
@@ -1961,8 +1962,7 @@ static int isp_attach_iommu(struct isp_device *isp)
 	mapping = arm_iommu_create_mapping(&platform_bus_type, SZ_1G, SZ_2G);
 	if (IS_ERR(mapping)) {
 		dev_err(isp->dev, "failed to create ARM IOMMU mapping\n");
-		ret = PTR_ERR(mapping);
-		goto error;
+		return PTR_ERR(mapping);
 	}
 
 	isp->mapping = mapping;

@@ -1977,7 +1977,8 @@ static int isp_attach_iommu(struct isp_device *isp)
 	return 0;
 
 error:
-	isp_detach_iommu(isp);
+	arm_iommu_release_mapping(isp->mapping);
+	isp->mapping = NULL;
 	return ret;
 }
#4 lore raw

PATCH v2 1/1 media: omap3isp: fix unbalanced dma_iommu_mapping

Quoted 14 lines · Suman Anna

Hi Mauro,

Quoted 11 lines · Laurent Pinchart

Hi Suman,

Thank you for the patch.

Quoted 6 lines · On Wednesday, 14 March 2018 17:41:36 EET Suman Anna wrote:

The OMAP3 ISP driver manages its MMU mappings through the IOMMU-aware ARM DMA backend. The current code creates a dma_iommu_mapping and attaches this to the ISP device, but never detaches the mapping in either the probe failure paths or the driver remove path resulting in an unbalanced mapping refcount and a memory leak. Fix this properly.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Reported-by Pavel Machek <[email protected]>
Signed-off-by Suman Anna <[email protected]>
Acked-by Sakari Ailus <[email protected]>
Reviewed-by Laurent Pinchart <[email protected]>

I don't see this patch in -next yet, can you pick up this patch at your earliest?

Here:

<URL:https://patchwork.linuxtv.org/patch/48599/>

Signature · 2 lines
Sakari Ailus
e-mail: sakari.ailus at iki.fi