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

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