drm/exynos: fix wrong pointer access at vm close.

Commit Message

Inki Dae Feb. 12, 2013, 12:13 p.m. UTC

From: YoungJun Cho <[e-mail]>

This patch fixes wrong pointer access issue to filp->f_op and
filp->private_data.

The exynos_drm_gem_mmap_ioctl() changes filp->f_op and
filp->private_data temporarily and restore them to use
original ones in exynos_drm_gem_mmap_buffer() but there
was no lock between the changing and the restoring so
wrong pointer access to filp->f_op and filp->private_data
was induced by vm close callback.

So this patch uses mutex lock properly to resolve this issue.

Signed-off-by: YoungJun Cho <[e-mail]>
Signed-off-by: Inki Dae <[e-mail]>
Signed-off-by: Kyungmin Park <[e-mail]>
---
 drivers/gpu/drm/exynos/exynos_drm_gem.c |   33 ++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 12 deletions(-)

Patch

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 4731807..67e17ce 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -329,17 +329,11 @@  static struct drm_file *exynos_drm_find_drm_file(struct drm_device *drm_dev,
 {
 	struct drm_file *file_priv;
 
-	mutex_lock(&drm_dev->struct_mutex);
-
 	/* find current process's drm_file from filelist. */
-	list_for_each_entry(file_priv, &drm_dev->filelist, lhead) {
-		if (file_priv->filp == filp) {
-			mutex_unlock(&drm_dev->struct_mutex);
+	list_for_each_entry(file_priv, &drm_dev->filelist, lhead)
+		if (file_priv->filp == filp)
 			return file_priv;
-		}
-	}
 
-	mutex_unlock(&drm_dev->struct_mutex);
 	WARN_ON(1);
 
 	return ERR_PTR(-EFAULT);
@@ -400,9 +394,7 @@  static int exynos_drm_gem_mmap_buffer(struct file *filp,
 	 */
 	drm_gem_object_reference(obj);
 
-	mutex_lock(&drm_dev->struct_mutex);
 	drm_vm_open_locked(drm_dev, vma);
-	mutex_unlock(&drm_dev->struct_mutex);
 
 	return 0;
 }
@@ -432,6 +424,16 @@  int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data,
 	}
 
 	/*
+	 * We have to use gem object and its fops for specific mmaper,
+	 * but vm_mmap() can deliver only filp. So we have to change
+	 * filp->f_op and filp->private_data temporarily, then restore
+	 * again. So it is important to keep lock until restoration the
+	 * settings to prevent others from misuse of filp->f_op or
+	 * filp->private_data.
+	 */
+	mutex_lock(&dev->struct_mutex);
+
+	/*
 	 * Set specific mmper's fops. And it will be restored by
 	 * exynos_drm_gem_mmap_buffer to dev->driver->fops.
 	 * This is used to call specific mapper temporarily.
@@ -448,13 +450,20 @@  int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data,
 	addr = vm_mmap(file_priv->filp, 0, args->size,
 			PROT_READ | PROT_WRITE, MAP_SHARED, 0);
 
-	drm_gem_object_unreference_unlocked(obj);
+	drm_gem_object_unreference(obj);
 
 	if (IS_ERR((void *)addr)) {
-		file_priv->filp->private_data = file_priv;
+		/* check filp->f_op, filp->private_data are restored */
+		if (file_priv->filp->f_op == &exynos_drm_gem_fops) {
+			file_priv->filp->f_op = fops_get(dev->driver->fops);
+			file_priv->filp->private_data = file_priv;
+		}
+		mutex_unlock(&dev->struct_mutex);
 		return PTR_ERR((void *)addr);
 	}
 
+	mutex_unlock(&dev->struct_mutex);
+
 	args->mapped = addr;
 
 	DRM_DEBUG_KMS("mapped = 0x%lx\n", (unsigned long)args->mapped);