[PATCH 08/12] drm: don't de-authenticate clients on master-close

[Daniel Vetter]

On Wed, Jul 23, 2014 at 05:26:43PM +0200, David Herrmann wrote:
> If an active DRM-Master closes its device, we deauthenticate all clients
> on that master. However, if an inactive DRM-Master closes its device, we
> do nothing. This is quite inconsistent and breaks several scenarios:
> 
>  1) If this was used as security mechanism, it fails horribly if a master
>     closes a device while VT switched away. Furthermore, none of the few
>     drivers using ->master_*() callbacks seems to require it, anyway.
> 
>  2) If you spawn weston (or any other non-UMS compositor) in background
>     while another compositor is active, both will get assigned to the
>     same "drm_master" object. If the foreground compositor now exits, all
>     clients of both the foreground AND background compositor will be
>     de-authenticated leading to unexpected behavior.
> 
> Stop this non-sense and keep clients authenticated. We don't do this when
> dropping DRM-Master (i.e., switching VTs) so don't do it on active-close
> either!
> 
> Signed-off-by: David Herrmann <dh.herrmann at gmail.com>

Yeah, we have a bit a confusion about what master does. Iirc back in the
dri days the idea was that the master completely gates access for clients,
but we've long since moved away from this model (if it ever really was
fully implemented in the drm core). Dropping this and making master a pure
priv flag for certain compositor operations (modeset, flink_open, ...)
makes a lot of sense.

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

> ---
>  drivers/gpu/drm/drm_fops.c | 13 ++-----------
>  include/drm/drmP.h         |  1 -
>  2 files changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index f65087e..ff0a13f 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -252,8 +252,7 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)
>  	priv->minor = minor;
>  
>  	/* for compatibility root is always authenticated */
> -	priv->always_authenticated = capable(CAP_SYS_ADMIN);
> -	priv->authenticated = priv->always_authenticated;
> +	priv->authenticated = capable(CAP_SYS_ADMIN);
>  	priv->lock_count = 0;
>  
>  	INIT_LIST_HEAD(&priv->lhead);
> @@ -453,20 +452,12 @@ int drm_release(struct inode *inode, struct file *filp)
>  
>  	if (drm_is_master(file_priv)) {
>  		struct drm_master *master = file_priv->master;
> -		struct drm_file *temp;
> -
> -		mutex_lock(&dev->struct_mutex);
> -		list_for_each_entry(temp, &dev->filelist, lhead) {
> -			if ((temp->master == file_priv->master) &&
> -			    (temp != file_priv))
> -				temp->authenticated = temp->always_authenticated;
> -		}
>  
>  		/**
>  		 * Since the master is disappearing, so is the
>  		 * possibility to lock.
>  		 */
> -
> +		mutex_lock(&dev->struct_mutex);
>  		if (master->lock.hw_lock) {
>  			if (dev->sigdata.lock == master->lock.hw_lock)
>  				dev->sigdata.lock = NULL;
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index e1bb585..48d2fe7 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -385,7 +385,6 @@ struct drm_prime_file_private {
>  
>  /** File private data */
>  struct drm_file {
> -	unsigned always_authenticated :1;
>  	unsigned authenticated :1;
>  	/* true when the client has asked us to expose stereo 3D mode flags */
>  	unsigned stereo_allowed :1;
> -- 
> 2.0.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch