From 5af9f116e6a0e1de675da979a19f95a74ce2aa2e Mon Sep 17 00:00:00 2001 From: YoungJun Cho Date: Thu, 7 Feb 2013 16:17:54 +0900 Subject: [PATCH] drm/exynos: fix wrong pointer access at vm close. 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 Signed-off-by: Inki Dae Signed-off-by: Kyungmin Park --- drivers/gpu/drm/exynos/exynos_drm_gem.c | 33 ++++++++++++++++--------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c index 473180776528..67e17ce112b6 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; } @@ -431,6 +423,16 @@ int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data, return -EINVAL; } + /* + * 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. @@ -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);