[v2,1/5] drm/exynos: iommu: move dma_params configuration code to separate functions
diff mbox

Message ID 1466150067-8065-2-git-send-email-m.szyprowski@samsung.com
State Not Applicable
Headers show

Commit Message

Marek Szyprowski June 17, 2016, 7:54 a.m. UTC
Move code for managing DMA max segment size parameter to separate
functions. This patch also replaces devm_kzalloc() with kzalloc() and
adds proper kfree call. devm_kzalloc() cannot be used for dma_params
structure, because it will be freed on driver remove not on device
release. This means in case of Exynos DRM being compiled as module and
loaded 2 times, a user-after-free issue will happen.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_iommu.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

Comments

Inki Dae July 8, 2016, 8:15 a.m. UTC | #1
Hi Marek,

2016년 06월 17일 16:54에 Marek Szyprowski 이(가) 쓴 글:
> Move code for managing DMA max segment size parameter to separate
> functions. This patch also replaces devm_kzalloc() with kzalloc() and
> adds proper kfree call. devm_kzalloc() cannot be used for dma_params
> structure, because it will be freed on driver remove not on device
> release. This means in case of Exynos DRM being compiled as module and
> loaded 2 times, a user-after-free issue will happen.

Picked this patch series up.

Thanks,
Inki Dae

> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_iommu.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.c b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
> index 7ca09ee19656..1e82529e0c41 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_iommu.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
> @@ -21,6 +21,23 @@
>  #include "exynos_drm_drv.h"
>  #include "exynos_drm_iommu.h"
>  
> +static inline int configure_dma_max_seg_size(struct device *dev)
> +{
> +	if (!dev->dma_parms)
> +		dev->dma_parms = kzalloc(sizeof(*dev->dma_parms), GFP_KERNEL);
> +	if (!dev->dma_parms)
> +		return -ENOMEM;
> +
> +	dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
> +	return 0;
> +}
> +
> +static inline void clear_dma_max_seg_size(struct device *dev)
> +{
> +	kfree(dev->dma_parms);
> +	dev->dma_parms = NULL;
> +}
> +
>  /*
>   * drm_create_iommu_mapping - create a mapping structure
>   *
> @@ -80,13 +97,10 @@ int drm_iommu_attach_device(struct drm_device *drm_dev,
>  	if (!priv->mapping)
>  		return 0;
>  
> -	subdrv_dev->dma_parms = devm_kzalloc(subdrv_dev,
> -					sizeof(*subdrv_dev->dma_parms),
> -					GFP_KERNEL);
> -	if (!subdrv_dev->dma_parms)
> -		return -ENOMEM;
>  
> -	dma_set_max_seg_size(subdrv_dev, 0xffffffffu);
> +	ret = configure_dma_max_seg_size(subdrv_dev);
> +	if (ret)
> +		return ret;
>  
>  	if (subdrv_dev->archdata.mapping)
>  		arm_iommu_detach_device(subdrv_dev);
> @@ -94,6 +108,7 @@ int drm_iommu_attach_device(struct drm_device *drm_dev,
>  	ret = arm_iommu_attach_device(subdrv_dev, priv->mapping);
>  	if (ret < 0) {
>  		DRM_DEBUG_KMS("failed iommu attach.\n");
> +		clear_dma_max_seg_size(subdrv_dev);
>  		return ret;
>  	}
>  
> @@ -119,4 +134,5 @@ void drm_iommu_detach_device(struct drm_device *drm_dev,
>  		return;
>  
>  	arm_iommu_detach_device(subdrv_dev);
> +	clear_dma_max_seg_size(subdrv_dev);
>  }
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.c b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
index 7ca09ee19656..1e82529e0c41 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_iommu.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
@@ -21,6 +21,23 @@ 
 #include "exynos_drm_drv.h"
 #include "exynos_drm_iommu.h"
 
+static inline int configure_dma_max_seg_size(struct device *dev)
+{
+	if (!dev->dma_parms)
+		dev->dma_parms = kzalloc(sizeof(*dev->dma_parms), GFP_KERNEL);
+	if (!dev->dma_parms)
+		return -ENOMEM;
+
+	dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
+	return 0;
+}
+
+static inline void clear_dma_max_seg_size(struct device *dev)
+{
+	kfree(dev->dma_parms);
+	dev->dma_parms = NULL;
+}
+
 /*
  * drm_create_iommu_mapping - create a mapping structure
  *
@@ -80,13 +97,10 @@  int drm_iommu_attach_device(struct drm_device *drm_dev,
 	if (!priv->mapping)
 		return 0;
 
-	subdrv_dev->dma_parms = devm_kzalloc(subdrv_dev,
-					sizeof(*subdrv_dev->dma_parms),
-					GFP_KERNEL);
-	if (!subdrv_dev->dma_parms)
-		return -ENOMEM;
 
-	dma_set_max_seg_size(subdrv_dev, 0xffffffffu);
+	ret = configure_dma_max_seg_size(subdrv_dev);
+	if (ret)
+		return ret;
 
 	if (subdrv_dev->archdata.mapping)
 		arm_iommu_detach_device(subdrv_dev);
@@ -94,6 +108,7 @@  int drm_iommu_attach_device(struct drm_device *drm_dev,
 	ret = arm_iommu_attach_device(subdrv_dev, priv->mapping);
 	if (ret < 0) {
 		DRM_DEBUG_KMS("failed iommu attach.\n");
+		clear_dma_max_seg_size(subdrv_dev);
 		return ret;
 	}
 
@@ -119,4 +134,5 @@  void drm_iommu_detach_device(struct drm_device *drm_dev,
 		return;
 
 	arm_iommu_detach_device(subdrv_dev);
+	clear_dma_max_seg_size(subdrv_dev);
 }