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

From: Sean Paul
Date: Mon Oct 16 2017 - 16:35:07 EST

Next message: Mario.Limonciello: "RE: staging: rtsx: Add support for RTS5260"
Previous message: Mike Kravetz: "Re: [RFC PATCH 3/3] mm/map_contig: Add mmap(MAP_CONTIG) support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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@xxxxxxxx>:
>
> * 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@xxxxxxxxxx>
>
> * remove support for leasing encoders.
> * add support for leasing planes.
>
> Signed-off-by: Keith Packard <keithp@xxxxxxxxxx>
> Signed-off-by: Dave Airlie <airlied@xxxxxxxxxx>

Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx>

> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Sean Paul, Software Engineer, Google / Chromium OS

Next message: Mario.Limonciello: "RE: staging: rtsx: Add support for RTS5260"
Previous message: Mike Kravetz: "Re: [RFC PATCH 3/3] mm/map_contig: Add mmap(MAP_CONTIG) support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]