diff mbox

[4/5] drm: Check mode object lease status in all master ioctl paths [v3]

Message ID 20171013015631.6926-5-keithp@keithp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Keith Packard Oct. 13, 2017, 1:56 a.m. UTC 
Attempts to modify un-leased objects are rejected with an error.
Information returned about unleased objects is modified to make them
appear unusable and/or disconnected.

Changes for v2 as suggested by Daniel Vetter <daniel.vetter@ffwll.ch>:

 * With the change in the __drm_mode_object_find API to pass the
   file_priv along, we can now centralize most of the lease-based
   access checks in that function.

 * A few places skip that API and require in-line checks.

Changes for v3 provided by Dave Airlie <airlied@redhat.com>

 * remove support for leasing encoders.
 * add support for leasing planes.

Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_auth.c        |  2 +-
 drivers/gpu/drm/drm_encoder.c     |  5 +++--
 drivers/gpu/drm/drm_mode_config.c | 22 +++++++++++++---------
 drivers/gpu/drm/drm_mode_object.c | 22 ++++++++++++++++++++++
 drivers/gpu/drm/drm_plane.c       | 18 +++++++++++-------
 drivers/gpu/drm/drm_vblank.c      | 18 ++++++++++++++++--
 include/drm/drm_lease.h           |  2 --
 7 files changed, 66 insertions(+), 23 deletions(-)

Comments

Sean Paul Oct. 16, 2017, 8:34 p.m. UTC | #1 
On Thu, Oct 12, 2017 at 06:56:30PM -0700, Keith Packard wrote:
> Attempts to modify un-leased objects are rejected with an error.
> Information returned about unleased objects is modified to make them
> appear unusable and/or disconnected.
> 
> Changes for v2 as suggested by Daniel Vetter <daniel.vetter@ffwll.ch>:
> 
>  * With the change in the __drm_mode_object_find API to pass the
>    file_priv along, we can now centralize most of the lease-based
>    access checks in that function.
> 
>  * A few places skip that API and require in-line checks.
> 
> Changes for v3 provided by Dave Airlie <airlied@redhat.com>
> 
>  * remove support for leasing encoders.
>  * add support for leasing planes.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
>  drivers/gpu/drm/drm_auth.c        |  2 +-
>  drivers/gpu/drm/drm_encoder.c     |  5 +++--
>  drivers/gpu/drm/drm_mode_config.c | 22 +++++++++++++---------
>  drivers/gpu/drm/drm_mode_object.c | 22 ++++++++++++++++++++++
>  drivers/gpu/drm/drm_plane.c       | 18 +++++++++++-------
>  drivers/gpu/drm/drm_vblank.c      | 18 ++++++++++++++++--
>  include/drm/drm_lease.h           |  2 --
>  7 files changed, 66 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
> index 541177c71d51..bab26b477738 100644
> --- a/drivers/gpu/drm/drm_auth.c
> +++ b/drivers/gpu/drm/drm_auth.c
> @@ -310,7 +310,7 @@ void drm_master_release(struct drm_file *file_priv)
>   */
>  bool drm_is_current_master(struct drm_file *fpriv)
>  {
> -	return fpriv->is_master && fpriv->master == fpriv->minor->dev->master;
> +	return fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master;
>  }
>  EXPORT_SYMBOL(drm_is_current_master);
>  
> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> index 43f644844b83..59e0ebe733f8 100644
> --- a/drivers/gpu/drm/drm_encoder.c
> +++ b/drivers/gpu/drm/drm_encoder.c
> @@ -226,7 +226,7 @@ int drm_mode_getencoder(struct drm_device *dev, void *data,
>  
>  	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>  	crtc = drm_encoder_get_crtc(encoder);
> -	if (crtc)
> +	if (crtc && drm_lease_held(file_priv, crtc->base.id))
>  		enc_resp->crtc_id = crtc->base.id;
>  	else
>  		enc_resp->crtc_id = 0;
> @@ -234,7 +234,8 @@ int drm_mode_getencoder(struct drm_device *dev, void *data,
>  
>  	enc_resp->encoder_type = encoder->encoder_type;
>  	enc_resp->encoder_id = encoder->base.id;
> -	enc_resp->possible_crtcs = encoder->possible_crtcs;
> +	enc_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv,
> +							  encoder->possible_crtcs);
>  	enc_resp->possible_clones = encoder->possible_clones;
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 919e78d45ab0..cda8bfab6d3b 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -122,10 +122,12 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
>  	count = 0;
>  	crtc_id = u64_to_user_ptr(card_res->crtc_id_ptr);
>  	drm_for_each_crtc(crtc, dev) {
> -		if (count < card_res->count_crtcs &&
> -		    put_user(crtc->base.id, crtc_id + count))
> -			return -EFAULT;
> -		count++;
> +		if (drm_lease_held(file_priv, crtc->base.id)) {
> +			if (count < card_res->count_crtcs &&
> +			    put_user(crtc->base.id, crtc_id + count))
> +				return -EFAULT;
> +			count++;
> +		}
>  	}
>  	card_res->count_crtcs = count;
>  
> @@ -143,12 +145,14 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
>  	count = 0;
>  	connector_id = u64_to_user_ptr(card_res->connector_id_ptr);
>  	drm_for_each_connector_iter(connector, &conn_iter) {
> -		if (count < card_res->count_connectors &&
> -		    put_user(connector->base.id, connector_id + count)) {
> -			drm_connector_list_iter_end(&conn_iter);
> -			return -EFAULT;
> +		if (drm_lease_held(file_priv, connector->base.id)) {
> +			if (count < card_res->count_connectors &&
> +			    put_user(connector->base.id, connector_id + count)) {
> +				drm_connector_list_iter_end(&conn_iter);
> +				return -EFAULT;
> +			}
> +			count++;
>  		}
> -		count++;
>  	}
>  	card_res->count_connectors = count;
>  	drm_connector_list_iter_end(&conn_iter);
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index 240a05d91a53..d1599f36b605 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -104,6 +104,25 @@ void drm_mode_object_unregister(struct drm_device *dev,
>  	mutex_unlock(&dev->mode_config.idr_mutex);
>  }
>  
> +/**
> + * drm_lease_required - check types which must be leased to be used
> + * @type: type of object
> + *
> + * Returns whether the provided type of drm_mode_object must
> + * be owned or leased to be used by a process.
> + */
> +static bool drm_lease_required(uint32_t type)
> +{
> +	switch(type) {
> +	case DRM_MODE_OBJECT_CRTC:
> +	case DRM_MODE_OBJECT_CONNECTOR:
> +	case DRM_MODE_OBJECT_PLANE:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>  struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
>  					       struct drm_file *file_priv,
>  					       uint32_t id, uint32_t type)
> @@ -117,6 +136,9 @@ struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
>  	if (obj && obj->id != id)
>  		obj = NULL;
>  
> +	if (obj && drm_lease_required(obj->type) && !_drm_lease_held(file_priv, obj->id))
> +		obj = NULL;
> +
>  	if (obj && obj->free_cb) {
>  		if (!kref_get_unless_zero(&obj->refcount))
>  			obj = NULL;
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index c10869bad729..20401baf5909 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -479,10 +479,12 @@ int drm_mode_getplane_res(struct drm_device *dev, void *data,
>  		    !file_priv->universal_planes)
>  			continue;
>  
> -		if (count < config->num_total_plane &&
> -		    put_user(plane->base.id, plane_ptr + count))
> -			return -EFAULT;
> -		count++;
> +		if (drm_lease_held(file_priv, plane->base.id)) {
> +			if (count < config->num_total_plane &&
> +			    put_user(plane->base.id, plane_ptr + count))
> +				return -EFAULT;
> +			count++;
> +		}
>  	}
>  	plane_resp->count_planes = count;
>  
> @@ -504,9 +506,9 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
>  		return -ENOENT;
>  
>  	drm_modeset_lock(&plane->mutex, NULL);
> -	if (plane->state && plane->state->crtc)
> +	if (plane->state && plane->state->crtc && drm_lease_held(file_priv, plane->state->crtc->base.id))
>  		plane_resp->crtc_id = plane->state->crtc->base.id;
> -	else if (!plane->state && plane->crtc)
> +	else if (!plane->state && plane->crtc && drm_lease_held(file_priv, plane->crtc->base.id))
>  		plane_resp->crtc_id = plane->crtc->base.id;
>  	else
>  		plane_resp->crtc_id = 0;
> @@ -520,7 +522,9 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
>  	drm_modeset_unlock(&plane->mutex);
>  
>  	plane_resp->plane_id = plane->base.id;
> -	plane_resp->possible_crtcs = plane->possible_crtcs;
> +	plane_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv,
> +							    plane->possible_crtcs);
> +
>  	plane_resp->gamma_size = 0;
>  
>  	/*
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index deb080f62a29..4e4569679f44 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -1447,10 +1447,12 @@ static void drm_wait_vblank_reply(struct drm_device *dev, unsigned int pipe,
>  int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
>  			  struct drm_file *file_priv)
>  {
> +	struct drm_crtc *crtc;
>  	struct drm_vblank_crtc *vblank;
>  	union drm_wait_vblank *vblwait = data;
>  	int ret;
>  	u64 req_seq, seq;
> +	unsigned int pipe_index;
>  	unsigned int flags, pipe, high_pipe;
>  
>  	if (!dev->irq_enabled)
> @@ -1472,9 +1474,21 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
>  	flags = vblwait->request.type & _DRM_VBLANK_FLAGS_MASK;
>  	high_pipe = (vblwait->request.type & _DRM_VBLANK_HIGH_CRTC_MASK);
>  	if (high_pipe)
> -		pipe = high_pipe >> _DRM_VBLANK_HIGH_CRTC_SHIFT;
> +		pipe_index = high_pipe >> _DRM_VBLANK_HIGH_CRTC_SHIFT;
>  	else
> -		pipe = flags & _DRM_VBLANK_SECONDARY ? 1 : 0;
> +		pipe_index = flags & _DRM_VBLANK_SECONDARY ? 1 : 0;
> +
> +	/* Convert lease-relative crtc index into global crtc index */
> +	pipe = 0;
> +	drm_for_each_crtc(crtc, dev) {
> +		if (drm_lease_held(file_priv, crtc->base.id)) {
> +			if (pipe_index == 0)
> +				break;
> +			pipe_index--;
> +		}
> +		pipe++;
> +	}
> +
>  	if (pipe >= dev->num_crtcs)
>  		return -EINVAL;
>  
> diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h
> index a49667db1d6d..0981631b9aed 100644
> --- a/include/drm/drm_lease.h
> +++ b/include/drm/drm_lease.h
> @@ -31,6 +31,4 @@ void _drm_lease_revoke(struct drm_master *master);
>  
>  uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv, uint32_t crtcs);
>  
> -uint32_t drm_lease_filter_encoders(struct drm_file *file_priv, uint32_t encoders);
> -
>  #endif /* _DRM_LEASE_H_ */
> -- 
> 2.15.0.rc0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 541177c71d51..bab26b477738 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -310,7 +310,7 @@  void drm_master_release(struct drm_file *file_priv)
  */
 bool drm_is_current_master(struct drm_file *fpriv)
 {
-	return fpriv->is_master && fpriv->master == fpriv->minor->dev->master;
+	return fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master;
 }
 EXPORT_SYMBOL(drm_is_current_master);
 
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 43f644844b83..59e0ebe733f8 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -226,7 +226,7 @@  int drm_mode_getencoder(struct drm_device *dev, void *data,
 
 	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 	crtc = drm_encoder_get_crtc(encoder);
-	if (crtc)
+	if (crtc && drm_lease_held(file_priv, crtc->base.id))
 		enc_resp->crtc_id = crtc->base.id;
 	else
 		enc_resp->crtc_id = 0;
@@ -234,7 +234,8 @@  int drm_mode_getencoder(struct drm_device *dev, void *data,
 
 	enc_resp->encoder_type = encoder->encoder_type;
 	enc_resp->encoder_id = encoder->base.id;
-	enc_resp->possible_crtcs = encoder->possible_crtcs;
+	enc_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv,
+							  encoder->possible_crtcs);
 	enc_resp->possible_clones = encoder->possible_clones;
 
 	return 0;
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 919e78d45ab0..cda8bfab6d3b 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -122,10 +122,12 @@  int drm_mode_getresources(struct drm_device *dev, void *data,
 	count = 0;
 	crtc_id = u64_to_user_ptr(card_res->crtc_id_ptr);
 	drm_for_each_crtc(crtc, dev) {
-		if (count < card_res->count_crtcs &&
-		    put_user(crtc->base.id, crtc_id + count))
-			return -EFAULT;
-		count++;
+		if (drm_lease_held(file_priv, crtc->base.id)) {
+			if (count < card_res->count_crtcs &&
+			    put_user(crtc->base.id, crtc_id + count))
+				return -EFAULT;
+			count++;
+		}
 	}
 	card_res->count_crtcs = count;
 
@@ -143,12 +145,14 @@  int drm_mode_getresources(struct drm_device *dev, void *data,
 	count = 0;
 	connector_id = u64_to_user_ptr(card_res->connector_id_ptr);
 	drm_for_each_connector_iter(connector, &conn_iter) {
-		if (count < card_res->count_connectors &&
-		    put_user(connector->base.id, connector_id + count)) {
-			drm_connector_list_iter_end(&conn_iter);
-			return -EFAULT;
+		if (drm_lease_held(file_priv, connector->base.id)) {
+			if (count < card_res->count_connectors &&
+			    put_user(connector->base.id, connector_id + count)) {
+				drm_connector_list_iter_end(&conn_iter);
+				return -EFAULT;
+			}
+			count++;
 		}
-		count++;
 	}
 	card_res->count_connectors = count;
 	drm_connector_list_iter_end(&conn_iter);
diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
index 240a05d91a53..d1599f36b605 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -104,6 +104,25 @@  void drm_mode_object_unregister(struct drm_device *dev,
 	mutex_unlock(&dev->mode_config.idr_mutex);
 }
 
+/**
+ * drm_lease_required - check types which must be leased to be used
+ * @type: type of object
+ *
+ * Returns whether the provided type of drm_mode_object must
+ * be owned or leased to be used by a process.
+ */
+static bool drm_lease_required(uint32_t type)
+{
+	switch(type) {
+	case DRM_MODE_OBJECT_CRTC:
+	case DRM_MODE_OBJECT_CONNECTOR:
+	case DRM_MODE_OBJECT_PLANE:
+		return true;
+	default:
+		return false;
+	}
+}
+
 struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
 					       struct drm_file *file_priv,
 					       uint32_t id, uint32_t type)
@@ -117,6 +136,9 @@  struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
 	if (obj && obj->id != id)
 		obj = NULL;
 
+	if (obj && drm_lease_required(obj->type) && !_drm_lease_held(file_priv, obj->id))
+		obj = NULL;
+
 	if (obj && obj->free_cb) {
 		if (!kref_get_unless_zero(&obj->refcount))
 			obj = NULL;
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index c10869bad729..20401baf5909 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -479,10 +479,12 @@  int drm_mode_getplane_res(struct drm_device *dev, void *data,
 		    !file_priv->universal_planes)
 			continue;
 
-		if (count < config->num_total_plane &&
-		    put_user(plane->base.id, plane_ptr + count))
-			return -EFAULT;
-		count++;
+		if (drm_lease_held(file_priv, plane->base.id)) {
+			if (count < config->num_total_plane &&
+			    put_user(plane->base.id, plane_ptr + count))
+				return -EFAULT;
+			count++;
+		}
 	}
 	plane_resp->count_planes = count;
 
@@ -504,9 +506,9 @@  int drm_mode_getplane(struct drm_device *dev, void *data,
 		return -ENOENT;
 
 	drm_modeset_lock(&plane->mutex, NULL);
-	if (plane->state && plane->state->crtc)
+	if (plane->state && plane->state->crtc && drm_lease_held(file_priv, plane->state->crtc->base.id))
 		plane_resp->crtc_id = plane->state->crtc->base.id;
-	else if (!plane->state && plane->crtc)
+	else if (!plane->state && plane->crtc && drm_lease_held(file_priv, plane->crtc->base.id))
 		plane_resp->crtc_id = plane->crtc->base.id;
 	else
 		plane_resp->crtc_id = 0;
@@ -520,7 +522,9 @@  int drm_mode_getplane(struct drm_device *dev, void *data,
 	drm_modeset_unlock(&plane->mutex);
 
 	plane_resp->plane_id = plane->base.id;
-	plane_resp->possible_crtcs = plane->possible_crtcs;
+	plane_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv,
+							    plane->possible_crtcs);
+
 	plane_resp->gamma_size = 0;
 
 	/*
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index deb080f62a29..4e4569679f44 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1447,10 +1447,12 @@  static void drm_wait_vblank_reply(struct drm_device *dev, unsigned int pipe,
 int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
 			  struct drm_file *file_priv)
 {
+	struct drm_crtc *crtc;
 	struct drm_vblank_crtc *vblank;
 	union drm_wait_vblank *vblwait = data;
 	int ret;
 	u64 req_seq, seq;
+	unsigned int pipe_index;
 	unsigned int flags, pipe, high_pipe;
 
 	if (!dev->irq_enabled)
@@ -1472,9 +1474,21 @@  int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
 	flags = vblwait->request.type & _DRM_VBLANK_FLAGS_MASK;
 	high_pipe = (vblwait->request.type & _DRM_VBLANK_HIGH_CRTC_MASK);
 	if (high_pipe)
-		pipe = high_pipe >> _DRM_VBLANK_HIGH_CRTC_SHIFT;
+		pipe_index = high_pipe >> _DRM_VBLANK_HIGH_CRTC_SHIFT;
 	else
-		pipe = flags & _DRM_VBLANK_SECONDARY ? 1 : 0;
+		pipe_index = flags & _DRM_VBLANK_SECONDARY ? 1 : 0;
+
+	/* Convert lease-relative crtc index into global crtc index */
+	pipe = 0;
+	drm_for_each_crtc(crtc, dev) {
+		if (drm_lease_held(file_priv, crtc->base.id)) {
+			if (pipe_index == 0)
+				break;
+			pipe_index--;
+		}
+		pipe++;
+	}
+
 	if (pipe >= dev->num_crtcs)
 		return -EINVAL;
 
diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h
index a49667db1d6d..0981631b9aed 100644
--- a/include/drm/drm_lease.h
+++ b/include/drm/drm_lease.h
@@ -31,6 +31,4 @@  void _drm_lease_revoke(struct drm_master *master);
 
 uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv, uint32_t crtcs);
 
-uint32_t drm_lease_filter_encoders(struct drm_file *file_priv, uint32_t encoders);
-
 #endif /* _DRM_LEASE_H_ */