From: YoungJun Cho yj44.cho@samsung.com

The drm_gem_mmap_obj() has to be protected with dev->struct_mutex, but some caller functions do not. So it adds mutex lock to missing callers and adds assertion to check whether drm_gem_mmap_obj() is called with mutex lock or not.

Signed-off-by: YoungJun Cho yj44.cho@samsung.com Signed-off-by: Seung-Woo Kim sw0312.kim@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com CC: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com CC: Rob Clark robdclark@gmail.com --- This patch is based on drm-next branch.

Changes since v1: - Use lockdep_assert_held() instead of mutex_is_locked() as Maarten commented - Fix commit message about assertion

drivers/gpu/drm/drm_gem.c | 4 ++++ drivers/gpu/drm/drm_gem_cma_helper.c | 3 +++ drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 3 +++ 3 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 4321713..34c0be7 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -661,6 +661,8 @@ EXPORT_SYMBOL(drm_gem_vm_close); * the GEM object is not looked up based on its fake offset. To implement the * DRM mmap operation, drivers should use the drm_gem_mmap() function. * + * NOTE: This function has to be protected with dev->struct_mutex + * * Return 0 or success or -EINVAL if the object size is smaller than the VMA * size, or if no gem_vm_ops are provided. */ @@ -669,6 +671,8 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, { struct drm_device *dev = obj->dev;

+ lockdep_assert_held(&dev->struct_mutex); + /* Check for valid size. */ if (obj_size < vma->vm_end - vma->vm_start) return -EINVAL; diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index 9efabce..ce06397 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -487,9 +487,12 @@ static int drm_gem_cma_dmabuf_mmap(struct dma_buf *dmabuf, { struct drm_gem_cma_object *cma_obj = dmabuf->priv; struct drm_gem_object *gem_obj = &cma_obj->base; + struct drm_device *dev = gem_obj->dev; int ret;

+ mutex_lock(&dev->struct_mutex); ret = drm_gem_mmap_obj(gem_obj, gem_obj->size, vma); + mutex_unlock(&dev->struct_mutex); if (ret < 0) return ret;

diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c index 3256693..4fcca8d 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c +++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c @@ -140,12 +140,15 @@ static int omap_gem_dmabuf_mmap(struct dma_buf *buffer, struct vm_area_struct *vma) { struct drm_gem_object *obj = buffer->priv; + struct drm_device *dev = obj->dev; int ret = 0;

if (WARN_ON(!obj->filp)) return -EINVAL;

+ mutex_lock(&dev->struct_mutex); ret = drm_gem_mmap_obj(obj, omap_gem_mmap_size(obj), vma); + mutex_unlock(&dev->struct_mutex); if (ret < 0) return ret;

Hi,

On Thursday 27 June 2013 08:39:58 Seung-Woo Kim wrote:

From: YoungJun Cho yj44.cho@samsung.com

The drm_gem_mmap_obj() has to be protected with dev->struct_mutex, but some caller functions do not. So it adds mutex lock to missing callers and adds assertion to check whether drm_gem_mmap_obj() is called with mutex lock or not.

Signed-off-by: YoungJun Cho yj44.cho@samsung.com Signed-off-by: Seung-Woo Kim sw0312.kim@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com CC: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com CC: Rob Clark robdclark@gmail.com

Thanks for the patch.

Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com

---

This patch is based on drm-next branch.

Changes since v1:

• Use lockdep_assert_held() instead of mutex_is_locked() as Maarten

commented

• Fix commit message about assertion

drivers/gpu/drm/drm_gem.c | 4 ++++ drivers/gpu/drm/drm_gem_cma_helper.c | 3 +++ drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 3 +++ 3 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 4321713..34c0be7 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -661,6 +661,8 @@ EXPORT_SYMBOL(drm_gem_vm_close);

• the GEM object is not looked up based on its fake offset. To implement

the * DRM mmap operation, drivers should use the drm_gem_mmap() function. *

• • NOTE: This function has to be protected with dev->struct_mutex
• • Return 0 or success or -EINVAL if the object size is smaller than the

VMA * size, or if no gem_vm_ops are provided. */ @@ -669,6 +671,8 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, { struct drm_device *dev = obj->dev;

• lockdep_assert_held(&dev->struct_mutex);
• /* Check for valid size. */ if (obj_size < vma->vm_end - vma->vm_start) return -EINVAL;

diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index 9efabce..ce06397 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -487,9 +487,12 @@ static int drm_gem_cma_dmabuf_mmap(struct dma_buf *dmabuf, { struct drm_gem_cma_object *cma_obj = dmabuf->priv; struct drm_gem_object *gem_obj = &cma_obj->base;

• struct drm_device *dev = gem_obj->dev; int ret;

• mutex_lock(&dev->struct_mutex); ret = drm_gem_mmap_obj(gem_obj, gem_obj->size, vma);

• mutex_unlock(&dev->struct_mutex); if (ret < 0) return ret;

diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c index 3256693..4fcca8d 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c +++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c @@ -140,12 +140,15 @@ static int omap_gem_dmabuf_mmap(struct dma_buf *buffer, struct vm_area_struct *vma) { struct drm_gem_object *obj = buffer->priv;

• struct drm_device *dev = obj->dev; int ret = 0;

  if (WARN_ON(!obj->filp)) return -EINVAL;

• mutex_lock(&dev->struct_mutex); ret = drm_gem_mmap_obj(obj, omap_gem_mmap_size(obj), vma);

• mutex_unlock(&dev->struct_mutex); if (ret < 0) return ret;