7 files changed, 113 insertions, 37 deletions

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 4af6a3d5c9a1..13a3d3426961 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -262,15 +262,21 @@ again:
 
         mutex_lock(&dev->mode_config.idr_mutex);
         ret = idr_get_new_above(&dev->mode_config.crtc_idr, obj, 1, &new_id);
+
+        if (!ret) {
+                /*
+                 * Set up the object linking under the protection of the idr
+                 * lock so that other users can't see inconsistent state.
+                 */
+                obj->id = new_id;
+                obj->type = obj_type;
+        }
         mutex_unlock(&dev->mode_config.idr_mutex);
+
         if (ret == -EAGAIN)
                 goto again;
-        else if (ret)
-                return ret;
 
-        obj->id = new_id;
-        obj->type = obj_type;
-        return 0;
+        return ret;
 }
 
 /**
@@ -312,6 +318,12 @@ EXPORT_SYMBOL(drm_mode_object_find);
  * Allocates an ID for the framebuffer's parent mode object, sets its mode
  * functions & device file and adds it to the master fd list.
  *
+ * IMPORTANT:
+ * This functions publishes the fb and makes it available for concurrent access
+ * by other users. Which means by this point the fb _must_ be fully set up -
+ * since all the fb attributes are invariant over its lifetime, no further
+ * locking but only correct reference counting is required.
+ *
  * RETURNS:
  * Zero on success, error code on failure.
  */
@@ -320,16 +332,20 @@ int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb,
 {
         int ret;
 
+        mutex_lock(&dev->mode_config.fb_lock);
         kref_init(&fb->refcount);
+        INIT_LIST_HEAD(&fb->filp_head);
+        fb->dev = dev;
+        fb->funcs = funcs;
 
         ret = drm_mode_object_get(dev, &fb->base, DRM_MODE_OBJECT_FB);
         if (ret)
-                return ret;
+                goto out;
 
-        fb->dev = dev;
-        fb->funcs = funcs;
         dev->mode_config.num_fb++;
         list_add(&fb->head, &dev->mode_config.fb_list);
+out:
+        mutex_unlock(&dev->mode_config.fb_lock);
 
         return 0;
 }
@@ -385,8 +401,10 @@ void drm_framebuffer_cleanup(struct drm_framebuffer *fb)
          * this.)
          */
         drm_mode_object_put(dev, &fb->base);
+        mutex_lock(&dev->mode_config.fb_lock);
         list_del(&fb->head);
         dev->mode_config.num_fb--;
+        mutex_unlock(&dev->mode_config.fb_lock);
 }
 EXPORT_SYMBOL(drm_framebuffer_cleanup);
 
@@ -406,6 +424,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
         int ret;
 
         WARN_ON(!drm_modeset_is_locked(dev));
+        WARN_ON(!list_empty(&fb->filp_head));
 
         /* remove from any CRTC */
         list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
@@ -432,8 +451,6 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
                 }
         }
 
-        list_del(&fb->filp_head);
-
         drm_framebuffer_unreference(fb);
 }
 EXPORT_SYMBOL(drm_framebuffer_remove);
@@ -989,6 +1006,7 @@ void drm_mode_config_init(struct drm_device *dev)
 {
         mutex_init(&dev->mode_config.mutex);
         mutex_init(&dev->mode_config.idr_mutex);
+        mutex_init(&dev->mode_config.fb_lock);
         INIT_LIST_HEAD(&dev->mode_config.fb_list);
         INIT_LIST_HEAD(&dev->mode_config.crtc_list);
         INIT_LIST_HEAD(&dev->mode_config.connector_list);
@@ -1091,6 +1109,9 @@ void drm_mode_config_cleanup(struct drm_device *dev)
                 drm_property_destroy(dev, property);
         }
 
+        /* Single-threaded teardown context, so it's not requied to grab the
+         * fb_lock to protect against concurrent fb_list access. Contrary, it
+         * would actually deadlock with the drm_framebuffer_cleanup function. */
         list_for_each_entry_safe(fb, fbt, &dev->mode_config.fb_list, head) {
                 drm_framebuffer_remove(fb);
         }
@@ -1220,8 +1241,8 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
         if (!drm_core_check_feature(dev, DRIVER_MODESET))
                 return -EINVAL;
 
-        drm_modeset_lock_all(dev);
 
+        mutex_lock(&file_priv->fbs_lock);
         /*
          * For the non-control nodes we need to limit the list of resources
          * by IDs in the group list for this node
@@ -1229,6 +1250,23 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
         list_for_each(lh, &file_priv->fbs)
                 fb_count++;
 
+        /* handle this in 4 parts */
+        /* FBs */
+        if (card_res->count_fbs >= fb_count) {
+                copied = 0;
+                fb_id = (uint32_t __user *)(unsigned long)card_res->fb_id_ptr;
+                list_for_each_entry(fb, &file_priv->fbs, filp_head) {
+                        if (put_user(fb->base.id, fb_id + copied)) {
+                                mutex_unlock(&file_priv->fbs_lock);
+                                return -EFAULT;
+                        }
+                        copied++;
+                }
+        }
+        card_res->count_fbs = fb_count;
+        mutex_unlock(&file_priv->fbs_lock);
+
+        drm_modeset_lock_all(dev);
         mode_group = &file_priv->master->minor->mode_group;
         if (file_priv->master->minor->type == DRM_MINOR_CONTROL) {
 
@@ -1252,21 +1290,6 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
         card_res->max_width = dev->mode_config.max_width;
         card_res->min_width = dev->mode_config.min_width;
 
-        /* handle this in 4 parts */
-        /* FBs */
-        if (card_res->count_fbs >= fb_count) {
-                copied = 0;
-                fb_id = (uint32_t __user *)(unsigned long)card_res->fb_id_ptr;
-                list_for_each_entry(fb, &file_priv->fbs, filp_head) {
-                        if (put_user(fb->base.id, fb_id + copied)) {
-                                ret = -EFAULT;
-                                goto out;
-                        }
-                        copied++;
-                }
-        }
-        card_res->count_fbs = fb_count;
-
         /* CRTCs */
         if (card_res->count_crtcs >= crtc_count) {
                 copied = 0;
@@ -1765,8 +1788,10 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
         }
         crtc = obj_to_crtc(obj);
 
+        mutex_lock(&dev->mode_config.fb_lock);
         obj = drm_mode_object_find(dev, plane_req->fb_id,
                                    DRM_MODE_OBJECT_FB);
+        mutex_unlock(&dev->mode_config.fb_lock);
         if (!obj) {
                 DRM_DEBUG_KMS("Unknown framebuffer ID %d\n",
                               plane_req->fb_id);
@@ -1908,8 +1933,10 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
                         }
                         fb = crtc->fb;
                 } else {
+                        mutex_lock(&dev->mode_config.fb_lock);
                         obj = drm_mode_object_find(dev, crtc_req->fb_id,
                                                    DRM_MODE_OBJECT_FB);
+                        mutex_unlock(&dev->mode_config.fb_lock);
                         if (!obj) {
                                 DRM_DEBUG_KMS("Unknown FB ID%d\n",
                                                 crtc_req->fb_id);
@@ -2151,16 +2178,17 @@ int drm_mode_addfb(struct drm_device *dev,
         fb = dev->mode_config.funcs->fb_create(dev, file_priv, &r);
         if (IS_ERR(fb)) {
                 DRM_DEBUG_KMS("could not create framebuffer\n");
-                ret = PTR_ERR(fb);
-                goto out;
+                drm_modeset_unlock_all(dev);
+                return PTR_ERR(fb);
         }
 
+        mutex_lock(&file_priv->fbs_lock);
         or->fb_id = fb->base.id;
         list_add(&fb->filp_head, &file_priv->fbs);
         DRM_DEBUG_KMS("[FB:%d]\n", fb->base.id);
-
-out:
+        mutex_unlock(&file_priv->fbs_lock);
         drm_modeset_unlock_all(dev);
+
         return ret;
 }
 
@@ -2333,16 +2361,18 @@ int drm_mode_addfb2(struct drm_device *dev,
         fb = dev->mode_config.funcs->fb_create(dev, file_priv, r);
         if (IS_ERR(fb)) {
                 DRM_DEBUG_KMS("could not create framebuffer\n");
-                ret = PTR_ERR(fb);
-                goto out;
+                drm_modeset_unlock_all(dev);
+                return PTR_ERR(fb);
         }
 
+        mutex_lock(&file_priv->fbs_lock);
         r->fb_id = fb->base.id;
         list_add(&fb->filp_head, &file_priv->fbs);
         DRM_DEBUG_KMS("[FB:%d]\n", fb->base.id);
+        mutex_unlock(&file_priv->fbs_lock);
 
-out:
         drm_modeset_unlock_all(dev);
+
         return ret;
 }
 
@@ -2373,27 +2403,34 @@ int drm_mode_rmfb(struct drm_device *dev,
                 return -EINVAL;
 
         drm_modeset_lock_all(dev);
+        mutex_lock(&dev->mode_config.fb_lock);
         obj = drm_mode_object_find(dev, *id, DRM_MODE_OBJECT_FB);
         /* TODO check that we really get a framebuffer back. */
         if (!obj) {
+                mutex_unlock(&dev->mode_config.fb_lock);
                 ret = -EINVAL;
                 goto out;
         }
         fb = obj_to_fb(obj);
+        mutex_unlock(&dev->mode_config.fb_lock);
 
+        mutex_lock(&file_priv->fbs_lock);
         list_for_each_entry(fbl, &file_priv->fbs, filp_head)
                 if (fb == fbl)
                         found = 1;
-
         if (!found) {
                 ret = -EINVAL;
+                mutex_unlock(&file_priv->fbs_lock);
                 goto out;
         }
 
-        drm_framebuffer_remove(fb);
+        list_del_init(&fb->filp_head);
+        mutex_unlock(&file_priv->fbs_lock);
 
+        drm_framebuffer_remove(fb);
 out:
         drm_modeset_unlock_all(dev);
+
         return ret;
 }
 
@@ -2422,7 +2459,9 @@ int drm_mode_getfb(struct drm_device *dev,
                 return -EINVAL;
 
         drm_modeset_lock_all(dev);
+        mutex_lock(&dev->mode_config.fb_lock);
         obj = drm_mode_object_find(dev, r->fb_id, DRM_MODE_OBJECT_FB);
+        mutex_unlock(&dev->mode_config.fb_lock);
         if (!obj) {
                 ret = -EINVAL;
                 goto out;
@@ -2460,7 +2499,9 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
                 return -EINVAL;
 
         drm_modeset_lock_all(dev);
+        mutex_lock(&dev->mode_config.fb_lock);
         obj = drm_mode_object_find(dev, r->fb_id, DRM_MODE_OBJECT_FB);
+        mutex_unlock(&dev->mode_config.fb_lock);
         if (!obj) {
                 ret = -EINVAL;
                 goto out_err1;
@@ -2535,9 +2576,12 @@ void drm_fb_release(struct drm_file *priv)
         struct drm_framebuffer *fb, *tfb;
 
         drm_modeset_lock_all(dev);
+        mutex_lock(&priv->fbs_lock);
         list_for_each_entry_safe(fb, tfb, &priv->fbs, filp_head) {
+                list_del_init(&fb->filp_head);
                 drm_framebuffer_remove(fb);
         }
+        mutex_unlock(&priv->fbs_lock);
         drm_modeset_unlock_all(dev);
 }
 
@@ -3542,7 +3586,9 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
         if (crtc->funcs->page_flip == NULL)
                 goto out;
 
+        mutex_lock(&dev->mode_config.fb_lock);
         obj = drm_mode_object_find(dev, page_flip->fb_id, DRM_MODE_OBJECT_FB);
+        mutex_unlock(&dev->mode_config.fb_lock);
         if (!obj)
                 goto out;
         fb = obj_to_fb(obj);
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 133b4132983e..13fdcd10a605 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -276,6 +276,7 @@ static int drm_open_helper(struct inode *inode, struct file *filp,
 
         INIT_LIST_HEAD(&priv->lhead);
         INIT_LIST_HEAD(&priv->fbs);
+        mutex_init(&priv->fbs_lock);
         INIT_LIST_HEAD(&priv->event_list);
         init_waitqueue_head(&priv->event_wait);
         priv->event_space = 4096; /* set aside 4k for event buffer */
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e6a11ca85eaf..a40c674a57be 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1374,7 +1374,9 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
                    fb->base.bits_per_pixel);
         describe_obj(m, fb->obj);
         seq_printf(m, "\n");
+        mutex_unlock(&dev->mode_config.mutex);
 
+        mutex_lock(&dev->mode_config.fb_lock);
         list_for_each_entry(fb, &dev->mode_config.fb_list, base.head) {
                 if (&fb->base == ifbdev->helper.fb)
                         continue;
@@ -1387,8 +1389,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
                 describe_obj(m, fb->obj);
                 seq_printf(m, "\n");
         }
-
-        mutex_unlock(&dev->mode_config.mutex);
+        mutex_unlock(&dev->mode_config.fb_lock);
 
         return 0;
 }
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
index a135498a1298..0d6a161b204b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
@@ -165,7 +165,9 @@ int vmw_present_ioctl(struct drm_device *dev, void *data,
 
         drm_modeset_lock_all(dev);
 
+        mutex_lock(&dev->mode_config.fb_lock);
         obj = drm_mode_object_find(dev, arg->fb_id, DRM_MODE_OBJECT_FB);
+        mutex_unlock(&dev->mode_config.fb_lock);
         if (!obj) {
                 DRM_ERROR("Invalid framebuffer id.\n");
                 ret = -EINVAL;
@@ -248,7 +250,9 @@ int vmw_present_readback_ioctl(struct drm_device *dev, void *data,
 
         drm_modeset_lock_all(dev);
 
+        mutex_lock(&dev->mode_config.fb_lock);
         obj = drm_mode_object_find(dev, arg->fb_id, DRM_MODE_OBJECT_FB);
+        mutex_unlock(&dev->mode_config.fb_lock);
         if (!obj) {
                 DRM_ERROR("Invalid framebuffer id.\n");
                 ret = -EINVAL;
diff --git a/drivers/staging/omapdrm/omap_debugfs.c b/drivers/staging/omapdrm/omap_debugfs.c
index 2f122e00b51d..e95540b3e2f6 100644
--- a/drivers/staging/omapdrm/omap_debugfs.c
+++ b/drivers/staging/omapdrm/omap_debugfs.c
@@ -72,6 +72,7 @@ static int fb_show(struct seq_file *m, void *arg)
         seq_printf(m, "fbcon ");
         omap_framebuffer_describe(priv->fbdev->fb, m);
 
+        mutex_lock(&dev->mode_config.fb_lock);
         list_for_each_entry(fb, &dev->mode_config.fb_list, head) {
                 if (fb == priv->fbdev->fb)
                         continue;
@@ -79,6 +80,7 @@ static int fb_show(struct seq_file *m, void *arg)
                 seq_printf(m, "user ");
                 omap_framebuffer_describe(fb, m);
         }
+        mutex_unlock(&dev->mode_config.fb_lock);
 
         mutex_unlock(&dev->struct_mutex);
         mutex_unlock(&dev->mode_config.mutex);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3c609abe8c80..e74731c1a912 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -446,7 +446,15 @@ struct drm_file {
         int is_master; /* this file private is a master for a minor */
         struct drm_master *master; /* master this node is currently associated with
                                       N.B. not always minor->master */
+
+        /**
+         * fbs - List of framebuffers associated with this file.
+         *
+         * Protected by fbs_lock. Note that the fbs list holds a reference on
+         * the fb object to prevent it from untimely disappearing.
+         */
         struct list_head fbs;
+        struct mutex fbs_lock;
 
         wait_queue_head_t event_wait;
         struct list_head event_list;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index c89b1161f0be..c35a807d7e5c 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -254,6 +254,10 @@ struct drm_framebuffer {
          * userspace perspective.
          */
         struct kref refcount;
+        /*
+         * Place on the dev->mode_config.fb_list, access protected by
+         * dev->mode_config.fb_lock.
+         */
         struct list_head head;
         struct drm_mode_object base;
         const struct drm_framebuffer_funcs *funcs;
@@ -780,8 +784,18 @@ struct drm_mode_config {
         struct mutex idr_mutex; /* for IDR management */
         struct idr crtc_idr; /* use this idr for all IDs, fb, crtc, connector, modes - just makes life easier */
         /* this is limited to one for now */
+
+
+        /**
+         * fb_lock - mutex to protect fb state
+         *
+         * Besides the global fb list his also protects the fbs list in the
+         * file_priv
+         */
+        struct mutex fb_lock;
         int num_fb;
         struct list_head fb_list;
+
         int num_connector;
         struct list_head connector_list;
         int num_encoder;