Avoid NULL master_priv access in i915 kernel driver
authorStuart Abercrombie <sabercrombie@chromium.org>
Fri, 10 May 2013 22:11:02 +0000 (15:11 -0700)
committerChromeBot <chrome-bot@google.com>
Sat, 11 May 2013 01:15:27 +0000 (18:15 -0700)
In several places, including the interrupt handler, the driver assumes
it can deref. dev->primary->master->driver_priv if dev->primary->master
is non-NULL.  This wasn't true if drm_open_helper was midway through, so
rearrange the initialization order.

It looks as if http://crbug.com/221684 was caused by this, although I
have no direct repro.  I can produce the same kernel crash by adding a
delay to drm_open_helper and unplugging the monitor at the right time.

v2: Address this in drm_open_helper instead of the various access points --
basically Stephane's fix.

BUG=chromium:221684
TEST=The monitor unplug scenario doesn't bring down Link
Change-Id: I545f79422577cfe4cdd96e430b6bc902ccb1cab3
Reviewed-on: https://gerrit.chromium.org/gerrit/50407
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>
Commit-Queue: Stuart Abercrombie <sabercrombie@chromium.org>
Tested-by: Stuart Abercrombie <sabercrombie@chromium.org>
drivers/gpu/drm/drm_fops.c

index 123de28..0f7a698 100644 (file)
@@ -284,9 +284,11 @@ static int drm_open_helper(struct inode *inode, struct file *filp,
        /* if there is no current master make this fd it */
        mutex_lock(&dev->struct_mutex);
        if (!priv->minor->master) {
-               /* create a new master */
-               priv->minor->master = drm_master_create(priv->minor);
-               if (!priv->minor->master) {
+               /* create a new master but don't assign it yet
+                * to ensure master->driver_priv is set up first
+                */
+               struct drm_master* master_ptr = drm_master_create(priv->minor);
+               if (!master_ptr) {
                        mutex_unlock(&dev->struct_mutex);
                        ret = -ENOMEM;
                        goto out_free;
@@ -294,7 +296,7 @@ static int drm_open_helper(struct inode *inode, struct file *filp,
 
                priv->is_master = 1;
                /* take another reference for the copy in the local file priv */
-               priv->master = drm_master_get(priv->minor->master);
+               priv->master = drm_master_get(master_ptr);
 
                priv->authenticated = 1;
 
@@ -304,7 +306,7 @@ static int drm_open_helper(struct inode *inode, struct file *filp,
                        if (ret) {
                                mutex_lock(&dev->struct_mutex);
                                /* drop both references if this fails */
-                               drm_master_put(&priv->minor->master);
+                               drm_master_put(&master_ptr);
                                drm_master_put(&priv->master);
                                mutex_unlock(&dev->struct_mutex);
                                goto out_free;
@@ -315,12 +317,13 @@ static int drm_open_helper(struct inode *inode, struct file *filp,
                        ret = dev->driver->master_set(dev, priv, true);
                        if (ret) {
                                /* drop both references if this fails */
-                               drm_master_put(&priv->minor->master);
+                               drm_master_put(&master_ptr);
                                drm_master_put(&priv->master);
                                mutex_unlock(&dev->struct_mutex);
                                goto out_free;
                        }
                }
+               priv->minor->master = master_ptr;
                mutex_unlock(&dev->struct_mutex);
        } else {
                /* get a reference to the master */