[PATCH 06/16] drm: Protect the master management with a drm_device::master_mutex v3

2014-03-28 Thread David Herrmann
Hi

On Fri, Mar 28, 2014 at 12:52 PM, Thomas Hellstrom
 wrote:
> Thanks. Want a Reviewed-By: or Acked-By: added?

Oh, sure:

Reviewed-by: David Herrmann 

Thanks
David


[PATCH 06/16] drm: Protect the master management with a drm_device::master_mutex v3

2014-03-28 Thread Thomas Hellstrom
On 03/28/2014 12:23 PM, David Herrmann wrote:
> Hi
>
> On Fri, Mar 28, 2014 at 10:34 AM, Thomas Hellstrom
>  wrote:
>> The master management was previously protected by the 
>> drm_device::struct_mutex.
>> In order to avoid locking order violations in a reworked dropped master
>> security check in the vmwgfx driver, break it out into a separate 
>> master_mutex.
>> Locking order is master_mutex -> struct_mutex.
>>
>> Also remove drm_master::blocked since it's not used.
>>
>> v2: Add an inline comment about what drm_device::master_mutex is protecting.
>> v3: Remove unneeded struct_mutex locks. Fix error returns in
>> drm_setmaster_ioctl().
> Thanks, looks much better now. Tested on hsw, so I'm fine with pushing
> this into 3.15.
>
> Thanks
> David
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Thanks. Want a Reviewed-By: or Acked-By: added?

/Thomas


[PATCH 06/16] drm: Protect the master management with a drm_device::master_mutex v3

2014-03-28 Thread David Herrmann
Hi

On Fri, Mar 28, 2014 at 10:34 AM, Thomas Hellstrom
 wrote:
> The master management was previously protected by the 
> drm_device::struct_mutex.
> In order to avoid locking order violations in a reworked dropped master
> security check in the vmwgfx driver, break it out into a separate 
> master_mutex.
> Locking order is master_mutex -> struct_mutex.
>
> Also remove drm_master::blocked since it's not used.
>
> v2: Add an inline comment about what drm_device::master_mutex is protecting.
> v3: Remove unneeded struct_mutex locks. Fix error returns in
> drm_setmaster_ioctl().

Thanks, looks much better now. Tested on hsw, so I'm fine with pushing
this into 3.15.

Thanks
David


[PATCH 06/16] drm: Protect the master management with a drm_device::master_mutex v3

2014-03-28 Thread Thomas Hellstrom
The master management was previously protected by the drm_device::struct_mutex.
In order to avoid locking order violations in a reworked dropped master
security check in the vmwgfx driver, break it out into a separate master_mutex.
Locking order is master_mutex -> struct_mutex.

Also remove drm_master::blocked since it's not used.

v2: Add an inline comment about what drm_device::master_mutex is protecting.
v3: Remove unneeded struct_mutex locks. Fix error returns in
drm_setmaster_ioctl().

Signed-off-by: Thomas Hellstrom 
Reviewed-by: Brian Paul 
---
 drivers/gpu/drm/drm_fops.c |   22 ++---
 drivers/gpu/drm/drm_stub.c |   43 ++---
 include/drm/drmP.h |   46 
 3 files changed, 64 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index c7792b1..a0ce39c 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -231,12 +231,11 @@ static int drm_open_helper(struct inode *inode, struct 
file *filp,

/* if there is no current master make this fd it, but do not create
 * any master object for render clients */
-   mutex_lock(>struct_mutex);
+   mutex_lock(>master_mutex);
if (drm_is_primary_client(priv) && !priv->minor->master) {
/* create a new master */
priv->minor->master = drm_master_create(priv->minor);
if (!priv->minor->master) {
-   mutex_unlock(>struct_mutex);
ret = -ENOMEM;
goto out_close;
}
@@ -244,29 +243,23 @@ 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->authenticated = 1;

-   mutex_unlock(>struct_mutex);
if (dev->driver->master_create) {
ret = dev->driver->master_create(dev, priv->master);
if (ret) {
-   mutex_lock(>struct_mutex);
/* drop both references if this fails */
drm_master_put(>minor->master);
drm_master_put(>master);
-   mutex_unlock(>struct_mutex);
goto out_close;
}
}
-   mutex_lock(>struct_mutex);
if (dev->driver->master_set) {
ret = dev->driver->master_set(dev, priv, true);
if (ret) {
/* drop both references if this fails */
drm_master_put(>minor->master);
drm_master_put(>master);
-   mutex_unlock(>struct_mutex);
goto out_close;
}
}
@@ -274,7 +267,7 @@ static int drm_open_helper(struct inode *inode, struct file 
*filp,
/* get a reference to the master */
priv->master = drm_master_get(priv->minor->master);
}
-   mutex_unlock(>struct_mutex);
+   mutex_unlock(>master_mutex);

mutex_lock(>struct_mutex);
list_add(>lhead, >filelist);
@@ -302,6 +295,7 @@ static int drm_open_helper(struct inode *inode, struct file 
*filp,
return 0;

 out_close:
+   mutex_unlock(>master_mutex);
if (dev->driver->postclose)
dev->driver->postclose(dev, priv);
 out_prime_destroy:
@@ -489,11 +483,13 @@ int drm_release(struct inode *inode, struct file *filp)
}
mutex_unlock(>ctxlist_mutex);

-   mutex_lock(>struct_mutex);
+   mutex_lock(>master_mutex);

if (file_priv->is_master) {
struct drm_master *master = file_priv->master;
struct drm_file *temp;
+
+   mutex_lock(>struct_mutex);
list_for_each_entry(temp, >filelist, lhead) {
if ((temp->master == file_priv->master) &&
(temp != file_priv))
@@ -512,6 +508,7 @@ int drm_release(struct inode *inode, struct file *filp)
master->lock.file_priv = NULL;
wake_up_interruptible_all(>lock.lock_queue);
}
+   mutex_unlock(>struct_mutex);

if (file_priv->minor->master == file_priv->master) {
/* drop the reference held my the minor */
@@ -521,10 +518,13 @@ int drm_release(struct inode *inode, struct file *filp)
}
}

-   /* drop the reference held my the file priv */
+   /* drop the master reference held by the file priv */
if (file_priv->master)