This patch resolves potential deadlock issue that can be incurred
by changing file->f_op and filp->private_data to exynos specific
mapper ops and gem object temporarily.

To resolve this issue, this patch creates a new anon file dedicated
to exynos specific mmaper, and making it used instead of existing one.

Signed-off-by: Inki Dae <inki.dae at samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.c |   21 +++++++++
 drivers/gpu/drm/exynos/exynos_drm_drv.h |    1 +
 drivers/gpu/drm/exynos/exynos_drm_gem.c |   74 ++++++-------------------------
 drivers/gpu/drm/exynos/exynos_drm_gem.h |    3 ++
 4 files changed, 38 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c 
b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 22b8f5e..b5e5957 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -14,6 +14,8 @@
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>

+#include <linux/anon_inodes.h>
+
 #include <drm/exynos_drm.h>

 #include "exynos_drm_drv.h"
@@ -150,9 +152,14 @@ static int exynos_drm_unload(struct drm_device *dev)
        return 0;
 }

+static const struct file_operations exynos_drm_gem_fops = {
+       .mmap = exynos_drm_gem_mmap_buffer,
+};
+
 static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
 {
        struct drm_exynos_file_private *file_priv;
+       struct file *anon_filp;
        int ret;

        file_priv = kzalloc(sizeof(*file_priv), GFP_KERNEL);
@@ -167,6 +174,16 @@ static int exynos_drm_open(struct drm_device *dev, struct 
drm_file *file)
                file->driver_priv = NULL;
        }

+       anon_filp = anon_inode_getfile("exynos_gem", &exynos_drm_gem_fops,
+                                       NULL, 0);
+       if (IS_ERR(anon_filp)) {
+               kfree(file_priv);
+               return PTR_ERR(anon_filp);
+       }
+
+       anon_filp->f_mode = FMODE_READ | FMODE_WRITE;
+       file_priv->anon_filp = anon_filp;
+
        return ret;
 }

@@ -179,6 +196,7 @@ static void exynos_drm_preclose(struct drm_device *dev,
 static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file)
 {
        struct exynos_drm_private *private = dev->dev_private;
+       struct drm_exynos_file_private *file_priv;
        struct drm_pending_vblank_event *v, *vt;
        struct drm_pending_event *e, *et;
        unsigned long flags;
@@ -204,6 +222,9 @@ static void exynos_drm_postclose(struct drm_device *dev, 
struct drm_file *file)
        }
        spin_unlock_irqrestore(&dev->event_lock, flags);

+       file_priv = file->driver_priv;
+       if (file_priv->anon_filp)
+               fput(file_priv->anon_filp);

        kfree(file->driver_priv);
        file->driver_priv = NULL;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h 
b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index eaa1966..0eaf5a2 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -226,6 +226,7 @@ struct exynos_drm_ipp_private {
 struct drm_exynos_file_private {
        struct exynos_drm_g2d_private   *g2d_priv;
        struct exynos_drm_ipp_private   *ipp_priv;
+       struct file                     *anon_filp;
 };

 /*
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c 
b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 1ade191..49b8c9b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -338,46 +338,22 @@ int exynos_drm_gem_map_offset_ioctl(struct drm_device 
*dev, void *data,
                        &args->offset);
 }

-static struct drm_file *exynos_drm_find_drm_file(struct drm_device *drm_dev,
-                                                       struct file *filp)
-{
-       struct drm_file *file_priv;
-
-       /* find current process's drm_file from filelist. */
-       list_for_each_entry(file_priv, &drm_dev->filelist, lhead)
-               if (file_priv->filp == filp)
-                       return file_priv;
-
-       WARN_ON(1);
-
-       return ERR_PTR(-EFAULT);
-}
-
-static int exynos_drm_gem_mmap_buffer(struct file *filp,
+int exynos_drm_gem_mmap_buffer(struct file *filp,
                                      struct vm_area_struct *vma)
 {
        struct drm_gem_object *obj = filp->private_data;
        struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
        struct drm_device *drm_dev = obj->dev;
        struct exynos_drm_gem_buf *buffer;
-       struct drm_file *file_priv;
        unsigned long vm_size;
        int ret;

+       WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
+
        vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
        vma->vm_private_data = obj;
        vma->vm_ops = drm_dev->driver->gem_vm_ops;

-       /* restore it to driver's fops. */
-       filp->f_op = fops_get(drm_dev->driver->fops);
-
-       file_priv = exynos_drm_find_drm_file(drm_dev, filp);
-       if (IS_ERR(file_priv))
-               return PTR_ERR(file_priv);
-
-       /* restore it to drm_file. */
-       filp->private_data = file_priv;
-
        update_vm_cache_attr(exynos_gem_obj, vma);

        vm_size = vma->vm_end - vma->vm_start;
@@ -411,15 +387,13 @@ static int exynos_drm_gem_mmap_buffer(struct file *filp,
        return 0;
 }

-static const struct file_operations exynos_drm_gem_fops = {
-       .mmap = exynos_drm_gem_mmap_buffer,
-};
-
 int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data,
                              struct drm_file *file_priv)
 {
+       struct drm_exynos_file_private *exynos_file_priv;
        struct drm_exynos_gem_mmap *args = data;
        struct drm_gem_object *obj;
+       struct file *anon_filp;
        unsigned long addr;

        if (!(dev->driver->driver_features & DRIVER_GEM)) {
@@ -427,47 +401,25 @@ int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, 
void *data,
                return -ENODEV;
        }

+       mutex_lock(&dev->struct_mutex);
+
        obj = drm_gem_object_lookup(dev, file_priv, args->handle);
        if (!obj) {
                DRM_ERROR("failed to lookup gem object.\n");
+               mutex_unlock(&dev->struct_mutex);
                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.
-        * This is used to call specific mapper temporarily.
-        */
-       file_priv->filp->f_op = &exynos_drm_gem_fops;
-
-       /*
-        * Set gem object to private_data so that specific mmaper
-        * can get the gem object. And it will be restored by
-        * exynos_drm_gem_mmap_buffer to drm_file.
-        */
-       file_priv->filp->private_data = obj;
+       exynos_file_priv = file_priv->driver_priv;
+       anon_filp = exynos_file_priv->anon_filp;
+       anon_filp->private_data = obj;

-       addr = vm_mmap(file_priv->filp, 0, args->size,
-                       PROT_READ | PROT_WRITE, MAP_SHARED, 0);
+       addr = vm_mmap(anon_filp, 0, args->size, PROT_READ | PROT_WRITE,
+                       MAP_SHARED, 0);

        drm_gem_object_unreference(obj);

        if (IS_ERR_VALUE(addr)) {
-               /* 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 (int)addr;
        }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h 
b/drivers/gpu/drm/exynos/exynos_drm_gem.h
index 702ec3a..fde860c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
@@ -122,6 +122,9 @@ int exynos_drm_gem_map_offset_ioctl(struct drm_device *dev, 
void *data,
 int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data,
                              struct drm_file *file_priv);

+int exynos_drm_gem_mmap_buffer(struct file *filp,
+                                     struct vm_area_struct *vma);
+
 /* map user space allocated by malloc to pages. */
 int exynos_drm_gem_userptr_ioctl(struct drm_device *dev, void *data,
                                      struct drm_file *file_priv);
-- 
1.7.9.5

Reply via email to