| Message ID | 20170401170841.2643-4-keithp@keithp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Apr 01, 2017 at 10:08:40AM -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. > > Signed-off-by: Keith Packard <keithp@keithp.com> Just a merge-technical idea to make this easier to change and less painful to rebase until we've locked down the semantics properly. And also because I expect we'll need v2 of this uapi soon or later anyway: I think it'd be good if we could consolidate all the lease checking into drm_mode_object_find (respectively __drm_mode_object_find). We'd need to wire up the fpriv to be able to do that, but we could upstream that patch right away before anything else. That should take care of most of the checks in this patch here. There's a few things on top: - filtering the various bitmasks. I think you have most, but we could perhaps upstream the helpers for these. - filtering object lists (essentially getresources and getplanes ioctls). - filtering implicit objects in the legacy ioctl. E.g. page_flip done through atomic doesn't just need the CRTC id, but also the id of the primary plane plus of the FB_ID atomic property. Similarly for all the other legacy ioctls. I think we want to make sure there's no difference here in behaviour. Especially for the last one it might be simplest to outright disallow all legacy ioctl and require that sub-drm_master nodes only get access to the read-only GET* ioctl (they get that anyway, even when they're not the current master), plus atomic. Makes it a _lot_ easier to implement. Downside is that amdgpu _really_ needs to land atomic asap :-) > --- > drivers/gpu/drm/drm_atomic.c | 3 +++ > drivers/gpu/drm/drm_auth.c | 2 +- > drivers/gpu/drm/drm_color_mgmt.c | 3 +++ > drivers/gpu/drm/drm_connector.c | 52 ++++++++++++++++++++++++++------------- > drivers/gpu/drm/drm_crtc.c | 15 ++++++++--- > drivers/gpu/drm/drm_encoder.c | 18 +++++++++++--- > drivers/gpu/drm/drm_mode_object.c | 3 +++ > drivers/gpu/drm/drm_plane.c | 36 +++++++++++++++++++++++---- > include/drm/drm_lease.h | 4 +++ > include/drm/drm_mode_object.h | 1 + > 10 files changed, 108 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index fdfb1ec17e66..a3cb95f7f1c1 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -2134,6 +2134,9 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, > goto out; > } > > + if ((ret = drm_lease_check(file_priv->master, obj->id)) < 0) > + goto out; > + > if (!obj->properties) { > drm_mode_object_unreference(obj); > ret = -ENOENT; > diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c > index 1db4f63860d1..44c99d12f4c1 100644 > --- a/drivers/gpu/drm/drm_auth.c > +++ b/drivers/gpu/drm/drm_auth.c > @@ -303,7 +303,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; This feels like it should be part of the earlier patch to make sub-drm_master objects for leasing. In that patch we should also make sure that all the current docs about drm_master are updated correctly (but maybe only later on, when we get towards merging this). This change here is pretty important wrt set/drop_master ioctl behaviour and leases. Cheers, Daniel > } > EXPORT_SYMBOL(drm_is_current_master); > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c > index 6543ebde501a..f8d7a499cf95 100644 > --- a/drivers/gpu/drm/drm_color_mgmt.c > +++ b/drivers/gpu/drm/drm_color_mgmt.c > @@ -206,6 +206,9 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev, > goto out; > } > > + if ((ret = drm_lease_check(file_priv->master, crtc->base.id)) < 0) > + goto out; > + > if (crtc->funcs->gamma_set == NULL) { > ret = -ENOSYS; > goto out; > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index 7a7019ac9388..a95db57dd966 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -1079,6 +1079,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, > struct drm_mode_modeinfo u_mode; > struct drm_mode_modeinfo __user *mode_ptr; > uint32_t __user *encoder_ptr; > + bool leased; > > if (!drm_core_check_feature(dev, DRIVER_MODESET)) > return -EINVAL; > @@ -1093,9 +1094,16 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, > goto out_unlock; > } > > - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) > - if (connector->encoder_ids[i] != 0) > - encoders_count++; > + leased = drm_lease_check(file_priv->master, connector->base.id) == 0; > + > + DRM_DEBUG_LEASE("connector %d leased %s\n", connector->base.id, leased ? "true" : "false"); > + > + if (leased) { > + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) > + if (connector->encoder_ids[i] != 0 && > + drm_lease_check(file_priv->master, connector->encoder_ids[i]) == 0) > + encoders_count++; > + } > > if (out_resp->count_modes == 0) { > connector->funcs->fill_modes(connector, > @@ -1104,21 +1112,29 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, > } > > /* delayed so we get modes regardless of pre-fill_modes state */ > - list_for_each_entry(mode, &connector->modes, head) > - if (drm_mode_expose_to_userspace(mode, file_priv)) > - mode_count++; > + if (leased) > + list_for_each_entry(mode, &connector->modes, head) > + if (drm_mode_expose_to_userspace(mode, file_priv)) > + mode_count++; > > out_resp->connector_id = connector->base.id; > out_resp->connector_type = connector->connector_type; > out_resp->connector_type_id = connector->connector_type_id; > - out_resp->mm_width = connector->display_info.width_mm; > - out_resp->mm_height = connector->display_info.height_mm; > - out_resp->subpixel = connector->display_info.subpixel_order; > - out_resp->connection = connector->status; > + if (leased) { > + out_resp->mm_width = connector->display_info.width_mm; > + out_resp->mm_height = connector->display_info.height_mm; > + out_resp->subpixel = connector->display_info.subpixel_order; > + out_resp->connection = connector->status; > + } else { > + out_resp->mm_width = 0; > + out_resp->mm_height = 0; > + out_resp->subpixel = 0; > + out_resp->connection = connector_status_disconnected; > + } > > drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); > encoder = drm_connector_get_encoder(connector); > - if (encoder) > + if (encoder && leased) > out_resp->encoder_id = encoder->base.id; > else > out_resp->encoder_id = 0; > @@ -1145,12 +1161,14 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, > } > out_resp->count_modes = mode_count; > > - ret = drm_mode_object_get_properties(&connector->base, file_priv->atomic, > - (uint32_t __user *)(unsigned long)(out_resp->props_ptr), > - (uint64_t __user *)(unsigned long)(out_resp->prop_values_ptr), > - &out_resp->count_props); > - if (ret) > - goto out; > + if (leased) { > + ret = drm_mode_object_get_properties(&connector->base, file_priv->atomic, > + (uint32_t __user *)(unsigned long)(out_resp->props_ptr), > + (uint64_t __user *)(unsigned long)(out_resp->prop_values_ptr), > + &out_resp->count_props); > + if (ret) > + goto out; > + } > > if ((out_resp->count_encoders >= encoders_count) && encoders_count) { > copied = 0; > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index e75f62cd8a65..95026ca74568 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -347,6 +347,7 @@ int drm_mode_getcrtc(struct drm_device *dev, > { > struct drm_mode_crtc *crtc_resp = data; > struct drm_crtc *crtc; > + bool leased; > > if (!drm_core_check_feature(dev, DRIVER_MODESET)) > return -EINVAL; > @@ -355,9 +356,13 @@ int drm_mode_getcrtc(struct drm_device *dev, > if (!crtc) > return -ENOENT; > > + leased = drm_lease_check(file_priv->master, crtc->base.id) == 0; > + > + DRM_DEBUG_LEASE("crtc %d leased %s\n", crtc->base.id, leased ? "true" : "false"); > + > drm_modeset_lock_crtc(crtc, crtc->primary); > crtc_resp->gamma_size = crtc->gamma_size; > - if (crtc->primary->fb) > + if (crtc->primary->fb && leased) > crtc_resp->fb_id = crtc->primary->fb->base.id; > else > crtc_resp->fb_id = 0; > @@ -365,7 +370,7 @@ int drm_mode_getcrtc(struct drm_device *dev, > if (crtc->state) { > crtc_resp->x = crtc->primary->state->src_x >> 16; > crtc_resp->y = crtc->primary->state->src_y >> 16; > - if (crtc->state->enable) { > + if (crtc->state->enable && leased) { > drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->state->mode); > crtc_resp->mode_valid = 1; > > @@ -375,7 +380,7 @@ int drm_mode_getcrtc(struct drm_device *dev, > } else { > crtc_resp->x = crtc->x; > crtc_resp->y = crtc->y; > - if (crtc->enabled) { > + if (crtc->enabled && leased) { > drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->mode); > crtc_resp->mode_valid = 1; > > @@ -529,6 +534,10 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, > } > DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name); > > + if ((ret = drm_lease_check(file_priv->master, crtc->base.id)) < 0) { > + DRM_DEBUG_KMS("CRTC lease not held\n"); > + goto out; > + } > if (crtc_req->mode_valid) { > /* If we have a mode we need a framebuffer. */ > /* If we pass -1, set the mode with the currently bound fb */ > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c > index 992879f15f23..24d03e13f522 100644 > --- a/drivers/gpu/drm/drm_encoder.c > +++ b/drivers/gpu/drm/drm_encoder.c > @@ -201,6 +201,7 @@ int drm_mode_getencoder(struct drm_device *dev, void *data, > struct drm_mode_get_encoder *enc_resp = data; > struct drm_encoder *encoder; > struct drm_crtc *crtc; > + bool leased; > > if (!drm_core_check_feature(dev, DRIVER_MODESET)) > return -EINVAL; > @@ -209,9 +210,13 @@ int drm_mode_getencoder(struct drm_device *dev, void *data, > if (!encoder) > return -ENOENT; > > + leased = drm_lease_check(file_priv->master, encoder->base.id) == 0; > + > + DRM_DEBUG_LEASE("encoder %d leased %s\n", encoder->base.id, leased ? "true" : "false"); > + > drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); > crtc = drm_encoder_get_crtc(encoder); > - if (crtc) > + if (crtc && leased) > enc_resp->crtc_id = crtc->base.id; > else > enc_resp->crtc_id = 0; > @@ -219,8 +224,15 @@ 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_clones = encoder->possible_clones; > + if (leased) { > + enc_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv->master, > + encoder->possible_crtcs); > + enc_resp->possible_clones = drm_lease_filter_encoders(file_priv->master, > + encoder->possible_clones); > + } else { > + enc_resp->possible_crtcs = 0; > + enc_resp->possible_clones = 0; > + } > > return 0; > } > diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c > index 9f17085b1fdd..9f8559d82a17 100644 > --- a/drivers/gpu/drm/drm_mode_object.c > +++ b/drivers/gpu/drm/drm_mode_object.c > @@ -404,6 +404,9 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, > goto out; > } > > + if ((ret = drm_lease_check(file_priv->master, arg->obj_id)) != 0) > + goto out; > + > if (!arg_obj->properties) > goto out_unref; > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > index 62b98f386fd1..df811869c1dd 100644 > --- a/drivers/gpu/drm/drm_plane.c > +++ b/drivers/gpu/drm/drm_plane.c > @@ -383,6 +383,7 @@ int drm_mode_getplane(struct drm_device *dev, void *data, > struct drm_mode_get_plane *plane_resp = data; > struct drm_plane *plane; > uint32_t __user *format_ptr; > + bool leased; > > if (!drm_core_check_feature(dev, DRIVER_MODESET)) > return -EINVAL; > @@ -391,27 +392,34 @@ int drm_mode_getplane(struct drm_device *dev, void *data, > if (!plane) > return -ENOENT; > > + leased = drm_lease_check(file_priv->master, plane->base.id) == 0; > + > drm_modeset_lock(&plane->mutex, NULL); > - if (plane->crtc) > + if (plane->crtc && leased) > plane_resp->crtc_id = plane->crtc->base.id; > else > plane_resp->crtc_id = 0; > > - if (plane->fb) > + if (plane->fb && leased) > plane_resp->fb_id = plane->fb->base.id; > else > plane_resp->fb_id = 0; > drm_modeset_unlock(&plane->mutex); > > plane_resp->plane_id = plane->base.id; > - plane_resp->possible_crtcs = plane->possible_crtcs; > + if (leased) > + plane_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv->master, > + plane->possible_crtcs); > + else > + plane_resp->possible_crtcs = 0; > + > plane_resp->gamma_size = 0; > > /* > * This ioctl is called twice, once to determine how much space is > * needed, and the 2nd time to fill it. > */ > - if (plane->format_count && > + if (plane->format_count && leased && > (plane_resp->count_format_types >= plane->format_count)) { > format_ptr = (uint32_t __user *)(unsigned long)plane_resp->format_type_ptr; > if (copy_to_user(format_ptr, > @@ -420,7 +428,10 @@ int drm_mode_getplane(struct drm_device *dev, void *data, > return -EFAULT; > } > } > - plane_resp->count_format_types = plane->format_count; > + if (leased) > + plane_resp->count_format_types = plane->format_count; > + else > + plane_resp->count_format_types = 0; > > return 0; > } > @@ -551,6 +562,7 @@ int drm_mode_setplane(struct drm_device *dev, void *data, > struct drm_plane *plane; > struct drm_crtc *crtc = NULL; > struct drm_framebuffer *fb = NULL; > + int ret; > > if (!drm_core_check_feature(dev, DRIVER_MODESET)) > return -EINVAL; > @@ -566,6 +578,12 @@ int drm_mode_setplane(struct drm_device *dev, void *data, > return -ENOENT; > } > > + if ((ret = drm_lease_check(file_priv->master, plane->base.id)) < 0) { > + DRM_DEBUG_KMS("Plane lease not held: %d error %d\n", > + plane->base.id, ret); > + return ret; > + } > + > if (plane_req->fb_id) { > fb = drm_framebuffer_lookup(dev, plane_req->fb_id); > if (!fb) { > @@ -687,6 +705,11 @@ static int drm_mode_cursor_common(struct drm_device *dev, > DRM_DEBUG_KMS("Unknown CRTC ID %d\n", req->crtc_id); > return -ENOENT; > } > + if ((ret = drm_lease_check(file_priv->master, crtc->base.id)) < 0) { > + DRM_DEBUG_KMS("CRTC lease not held %d error %d\n", > + crtc->base.id, ret); > + goto out; > + } > > /* > * If this crtc has a universal cursor plane, call that plane's update > @@ -785,6 +808,9 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, > if (!crtc) > return -ENOENT; > > + if ((ret = drm_lease_check(file_priv->master, crtc->base.id)) < 0) > + return ret; > + > if (crtc->funcs->page_flip_target) { > u32 current_vblank; > int r; > diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h > index e02adf3e42fd..8f91fc4226e3 100644 > --- a/include/drm/drm_lease.h > +++ b/include/drm/drm_lease.h > @@ -48,4 +48,8 @@ static inline int drm_lease_check(struct drm_master *master, int id) { > return 0; > } > > +uint32_t drm_lease_filter_crtcs(struct drm_master *master, uint32_t crtcs); > + > +uint32_t drm_lease_filter_encoders(struct drm_master *master, uint32_t encoders); > + > #endif /* _DRM_LEASE_H_ */ > diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h > index 43460b21d112..07830182598b 100644 > --- a/include/drm/drm_mode_object.h > +++ b/include/drm/drm_mode_object.h > @@ -24,6 +24,7 @@ > #define __DRM_MODESET_H__ > > #include <linux/kref.h> > +#include <drm/drm_lease.h> > struct drm_object_properties; > struct drm_property; > struct drm_device; > -- > 2.11.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Daniel Vetter <daniel@ffwll.ch> writes: > I think it'd be good if we could consolidate all the lease checking into > drm_mode_object_find (respectively __drm_mode_object_find). We'd need to > wire up the fpriv to be able to do that, but we could upstream that patch > right away before anything else. That should take care of most of the > checks in this patch here. That's a good idea. > There's a few things on top: > - filtering the various bitmasks. I think you have most, but we could > perhaps upstream the helpers for these. Yeah, would be nice to get hooks in place soon to avoid rebase adventures later. I guess that would involve shipping a stub drm_lease.h for now? > - filtering object lists (essentially getresources and getplanes ioctls). > - filtering implicit objects in the legacy ioctl. E.g. page_flip done > through atomic doesn't just need the CRTC id, but also the id of the > primary plane plus of the FB_ID atomic property. Similarly for all the > other legacy ioctls. I think we want to make sure there's no difference > here in behaviour. Oh, all of the implicit resource access from the legacy ioctls. Yeah, that will take a bit of research to identify all of them. > Especially for the last one it might be simplest to outright disallow all > legacy ioctl and require that sub-drm_master nodes only get access to the > read-only GET* ioctl (they get that anyway, even when they're not the > current master), plus atomic. Makes it a _lot_ easier to implement. > Downside is that amdgpu _really_ needs to land atomic asap :-) I'd like to avoid that particular dependency as amdgpu is something of a requirement for this particular project... I'll get started fixing the lease checking stuff to try and centralize it.
On Sun, Apr 02, 2017 at 09:37:16AM -0700, Keith Packard wrote: > Daniel Vetter <daniel@ffwll.ch> writes: > > > I think it'd be good if we could consolidate all the lease checking into > > drm_mode_object_find (respectively __drm_mode_object_find). We'd need to > > wire up the fpriv to be able to do that, but we could upstream that patch > > right away before anything else. That should take care of most of the > > checks in this patch here. > > That's a good idea. > > > There's a few things on top: > > - filtering the various bitmasks. I think you have most, but we could > > perhaps upstream the helpers for these. > > Yeah, would be nice to get hooks in place soon to avoid rebase > adventures later. I guess that would involve shipping a stub drm_lease.h > for now? I think we should see how far wiring drm_file *fpriv through the lookup functions gets us, but yeah we probably need a stub drm_lease.h for filtering. For the interface maybe pass a drm_file *fpriv for all the filtering functions, and deref into fpriv->master or whatever on the implementation. That gives us all the freedom to filter however we want really. > > - filtering object lists (essentially getresources and getplanes ioctls). > > - filtering implicit objects in the legacy ioctl. E.g. page_flip done > > through atomic doesn't just need the CRTC id, but also the id of the > > primary plane plus of the FB_ID atomic property. Similarly for all the > > other legacy ioctls. I think we want to make sure there's no difference > > here in behaviour. > > Oh, all of the implicit resource access from the legacy ioctls. Yeah, > that will take a bit of research to identify all of them. > > > Especially for the last one it might be simplest to outright disallow all > > legacy ioctl and require that sub-drm_master nodes only get access to the > > read-only GET* ioctl (they get that anyway, even when they're not the > > current master), plus atomic. Makes it a _lot_ easier to implement. > > Downside is that amdgpu _really_ needs to land atomic asap :-) > > I'd like to avoid that particular dependency as amdgpu is something of a > requirement for this particular project... Figured so :-) Otoh I've heard that atomic for amdgpu is happening for real now. Although Harry told me I need to wait a few more weeks before they're ready to present the rewritten stuff and get my feedback ... -Daniel
Daniel Vetter <daniel@ffwll.ch> writes: > I think it'd be good if we could consolidate all the lease checking into > drm_mode_object_find (respectively __drm_mode_object_find). We'd need to > wire up the fpriv to be able to do that, but we could upstream that patch > right away before anything else. That should take care of most of the > checks in this patch here. Hrm. Without some major changes, the effect of this will be to have the lessee see only the objects which it holds a lease on. I think that's OK as the lessor will be doing object filtering for its clients, however it's not what Dave and I discussed, so I just want to make him aware that the kernel is going to be simpler than originally planned. We had originally discussed having the lessee see the un-leased objects, but to have them reported with different status.
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index fdfb1ec17e66..a3cb95f7f1c1 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -2134,6 +2134,9 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, goto out; } + if ((ret = drm_lease_check(file_priv->master, obj->id)) < 0) + goto out; + if (!obj->properties) { drm_mode_object_unreference(obj); ret = -ENOENT; diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 1db4f63860d1..44c99d12f4c1 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -303,7 +303,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_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index 6543ebde501a..f8d7a499cf95 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -206,6 +206,9 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev, goto out; } + if ((ret = drm_lease_check(file_priv->master, crtc->base.id)) < 0) + goto out; + if (crtc->funcs->gamma_set == NULL) { ret = -ENOSYS; goto out; diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 7a7019ac9388..a95db57dd966 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1079,6 +1079,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, struct drm_mode_modeinfo u_mode; struct drm_mode_modeinfo __user *mode_ptr; uint32_t __user *encoder_ptr; + bool leased; if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EINVAL; @@ -1093,9 +1094,16 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, goto out_unlock; } - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) - if (connector->encoder_ids[i] != 0) - encoders_count++; + leased = drm_lease_check(file_priv->master, connector->base.id) == 0; + + DRM_DEBUG_LEASE("connector %d leased %s\n", connector->base.id, leased ? "true" : "false"); + + if (leased) { + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) + if (connector->encoder_ids[i] != 0 && + drm_lease_check(file_priv->master, connector->encoder_ids[i]) == 0) + encoders_count++; + } if (out_resp->count_modes == 0) { connector->funcs->fill_modes(connector, @@ -1104,21 +1112,29 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, } /* delayed so we get modes regardless of pre-fill_modes state */ - list_for_each_entry(mode, &connector->modes, head) - if (drm_mode_expose_to_userspace(mode, file_priv)) - mode_count++; + if (leased) + list_for_each_entry(mode, &connector->modes, head) + if (drm_mode_expose_to_userspace(mode, file_priv)) + mode_count++; out_resp->connector_id = connector->base.id; out_resp->connector_type = connector->connector_type; out_resp->connector_type_id = connector->connector_type_id; - out_resp->mm_width = connector->display_info.width_mm; - out_resp->mm_height = connector->display_info.height_mm; - out_resp->subpixel = connector->display_info.subpixel_order; - out_resp->connection = connector->status; + if (leased) { + out_resp->mm_width = connector->display_info.width_mm; + out_resp->mm_height = connector->display_info.height_mm; + out_resp->subpixel = connector->display_info.subpixel_order; + out_resp->connection = connector->status; + } else { + out_resp->mm_width = 0; + out_resp->mm_height = 0; + out_resp->subpixel = 0; + out_resp->connection = connector_status_disconnected; + } drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); encoder = drm_connector_get_encoder(connector); - if (encoder) + if (encoder && leased) out_resp->encoder_id = encoder->base.id; else out_resp->encoder_id = 0; @@ -1145,12 +1161,14 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, } out_resp->count_modes = mode_count; - ret = drm_mode_object_get_properties(&connector->base, file_priv->atomic, - (uint32_t __user *)(unsigned long)(out_resp->props_ptr), - (uint64_t __user *)(unsigned long)(out_resp->prop_values_ptr), - &out_resp->count_props); - if (ret) - goto out; + if (leased) { + ret = drm_mode_object_get_properties(&connector->base, file_priv->atomic, + (uint32_t __user *)(unsigned long)(out_resp->props_ptr), + (uint64_t __user *)(unsigned long)(out_resp->prop_values_ptr), + &out_resp->count_props); + if (ret) + goto out; + } if ((out_resp->count_encoders >= encoders_count) && encoders_count) { copied = 0; diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index e75f62cd8a65..95026ca74568 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -347,6 +347,7 @@ int drm_mode_getcrtc(struct drm_device *dev, { struct drm_mode_crtc *crtc_resp = data; struct drm_crtc *crtc; + bool leased; if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EINVAL; @@ -355,9 +356,13 @@ int drm_mode_getcrtc(struct drm_device *dev, if (!crtc) return -ENOENT; + leased = drm_lease_check(file_priv->master, crtc->base.id) == 0; + + DRM_DEBUG_LEASE("crtc %d leased %s\n", crtc->base.id, leased ? "true" : "false"); + drm_modeset_lock_crtc(crtc, crtc->primary); crtc_resp->gamma_size = crtc->gamma_size; - if (crtc->primary->fb) + if (crtc->primary->fb && leased) crtc_resp->fb_id = crtc->primary->fb->base.id; else crtc_resp->fb_id = 0; @@ -365,7 +370,7 @@ int drm_mode_getcrtc(struct drm_device *dev, if (crtc->state) { crtc_resp->x = crtc->primary->state->src_x >> 16; crtc_resp->y = crtc->primary->state->src_y >> 16; - if (crtc->state->enable) { + if (crtc->state->enable && leased) { drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->state->mode); crtc_resp->mode_valid = 1; @@ -375,7 +380,7 @@ int drm_mode_getcrtc(struct drm_device *dev, } else { crtc_resp->x = crtc->x; crtc_resp->y = crtc->y; - if (crtc->enabled) { + if (crtc->enabled && leased) { drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->mode); crtc_resp->mode_valid = 1; @@ -529,6 +534,10 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, } DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name); + if ((ret = drm_lease_check(file_priv->master, crtc->base.id)) < 0) { + DRM_DEBUG_KMS("CRTC lease not held\n"); + goto out; + } if (crtc_req->mode_valid) { /* If we have a mode we need a framebuffer. */ /* If we pass -1, set the mode with the currently bound fb */ diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c index 992879f15f23..24d03e13f522 100644 --- a/drivers/gpu/drm/drm_encoder.c +++ b/drivers/gpu/drm/drm_encoder.c @@ -201,6 +201,7 @@ int drm_mode_getencoder(struct drm_device *dev, void *data, struct drm_mode_get_encoder *enc_resp = data; struct drm_encoder *encoder; struct drm_crtc *crtc; + bool leased; if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EINVAL; @@ -209,9 +210,13 @@ int drm_mode_getencoder(struct drm_device *dev, void *data, if (!encoder) return -ENOENT; + leased = drm_lease_check(file_priv->master, encoder->base.id) == 0; + + DRM_DEBUG_LEASE("encoder %d leased %s\n", encoder->base.id, leased ? "true" : "false"); + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); crtc = drm_encoder_get_crtc(encoder); - if (crtc) + if (crtc && leased) enc_resp->crtc_id = crtc->base.id; else enc_resp->crtc_id = 0; @@ -219,8 +224,15 @@ 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_clones = encoder->possible_clones; + if (leased) { + enc_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv->master, + encoder->possible_crtcs); + enc_resp->possible_clones = drm_lease_filter_encoders(file_priv->master, + encoder->possible_clones); + } else { + enc_resp->possible_crtcs = 0; + enc_resp->possible_clones = 0; + } return 0; } diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c index 9f17085b1fdd..9f8559d82a17 100644 --- a/drivers/gpu/drm/drm_mode_object.c +++ b/drivers/gpu/drm/drm_mode_object.c @@ -404,6 +404,9 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, goto out; } + if ((ret = drm_lease_check(file_priv->master, arg->obj_id)) != 0) + goto out; + if (!arg_obj->properties) goto out_unref; diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 62b98f386fd1..df811869c1dd 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -383,6 +383,7 @@ int drm_mode_getplane(struct drm_device *dev, void *data, struct drm_mode_get_plane *plane_resp = data; struct drm_plane *plane; uint32_t __user *format_ptr; + bool leased; if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EINVAL; @@ -391,27 +392,34 @@ int drm_mode_getplane(struct drm_device *dev, void *data, if (!plane) return -ENOENT; + leased = drm_lease_check(file_priv->master, plane->base.id) == 0; + drm_modeset_lock(&plane->mutex, NULL); - if (plane->crtc) + if (plane->crtc && leased) plane_resp->crtc_id = plane->crtc->base.id; else plane_resp->crtc_id = 0; - if (plane->fb) + if (plane->fb && leased) plane_resp->fb_id = plane->fb->base.id; else plane_resp->fb_id = 0; drm_modeset_unlock(&plane->mutex); plane_resp->plane_id = plane->base.id; - plane_resp->possible_crtcs = plane->possible_crtcs; + if (leased) + plane_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv->master, + plane->possible_crtcs); + else + plane_resp->possible_crtcs = 0; + plane_resp->gamma_size = 0; /* * This ioctl is called twice, once to determine how much space is * needed, and the 2nd time to fill it. */ - if (plane->format_count && + if (plane->format_count && leased && (plane_resp->count_format_types >= plane->format_count)) { format_ptr = (uint32_t __user *)(unsigned long)plane_resp->format_type_ptr; if (copy_to_user(format_ptr, @@ -420,7 +428,10 @@ int drm_mode_getplane(struct drm_device *dev, void *data, return -EFAULT; } } - plane_resp->count_format_types = plane->format_count; + if (leased) + plane_resp->count_format_types = plane->format_count; + else + plane_resp->count_format_types = 0; return 0; } @@ -551,6 +562,7 @@ int drm_mode_setplane(struct drm_device *dev, void *data, struct drm_plane *plane; struct drm_crtc *crtc = NULL; struct drm_framebuffer *fb = NULL; + int ret; if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EINVAL; @@ -566,6 +578,12 @@ int drm_mode_setplane(struct drm_device *dev, void *data, return -ENOENT; } + if ((ret = drm_lease_check(file_priv->master, plane->base.id)) < 0) { + DRM_DEBUG_KMS("Plane lease not held: %d error %d\n", + plane->base.id, ret); + return ret; + } + if (plane_req->fb_id) { fb = drm_framebuffer_lookup(dev, plane_req->fb_id); if (!fb) { @@ -687,6 +705,11 @@ static int drm_mode_cursor_common(struct drm_device *dev, DRM_DEBUG_KMS("Unknown CRTC ID %d\n", req->crtc_id); return -ENOENT; } + if ((ret = drm_lease_check(file_priv->master, crtc->base.id)) < 0) { + DRM_DEBUG_KMS("CRTC lease not held %d error %d\n", + crtc->base.id, ret); + goto out; + } /* * If this crtc has a universal cursor plane, call that plane's update @@ -785,6 +808,9 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, if (!crtc) return -ENOENT; + if ((ret = drm_lease_check(file_priv->master, crtc->base.id)) < 0) + return ret; + if (crtc->funcs->page_flip_target) { u32 current_vblank; int r; diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h index e02adf3e42fd..8f91fc4226e3 100644 --- a/include/drm/drm_lease.h +++ b/include/drm/drm_lease.h @@ -48,4 +48,8 @@ static inline int drm_lease_check(struct drm_master *master, int id) { return 0; } +uint32_t drm_lease_filter_crtcs(struct drm_master *master, uint32_t crtcs); + +uint32_t drm_lease_filter_encoders(struct drm_master *master, uint32_t encoders); + #endif /* _DRM_LEASE_H_ */ diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h index 43460b21d112..07830182598b 100644 --- a/include/drm/drm_mode_object.h +++ b/include/drm/drm_mode_object.h @@ -24,6 +24,7 @@ #define __DRM_MODESET_H__ #include <linux/kref.h> +#include <drm/drm_lease.h> struct drm_object_properties; struct drm_property; struct drm_device;
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. Signed-off-by: Keith Packard <keithp@keithp.com> --- drivers/gpu/drm/drm_atomic.c | 3 +++ drivers/gpu/drm/drm_auth.c | 2 +- drivers/gpu/drm/drm_color_mgmt.c | 3 +++ drivers/gpu/drm/drm_connector.c | 52 ++++++++++++++++++++++++++------------- drivers/gpu/drm/drm_crtc.c | 15 ++++++++--- drivers/gpu/drm/drm_encoder.c | 18 +++++++++++--- drivers/gpu/drm/drm_mode_object.c | 3 +++ drivers/gpu/drm/drm_plane.c | 36 +++++++++++++++++++++++---- include/drm/drm_lease.h | 4 +++ include/drm/drm_mode_object.h | 1 + 10 files changed, 108 insertions(+), 29 deletions(-)