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

From: Keith Packard
Date: Sat Apr 01 2017 - 13:17:30 EST

Next message: Keith Packard: "[PATCH 4/4] drm: Add four ioctls for managing drm mode object leases"
Previous message: Keith Packard: "[PATCH 0/4] drm: Add mode resource leasing"
In reply to: Keith Packard: "[PATCH 1/4] drm: Add new LEASE debug level"
Next in thread: Daniel Vetter: "Re: [PATCH 3/4] drm: Check mode object lease status in all master ioctl paths"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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@xxxxxxxxxx>
---
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;
}
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

Next message: Keith Packard: "[PATCH 4/4] drm: Add four ioctls for managing drm mode object leases"
Previous message: Keith Packard: "[PATCH 0/4] drm: Add mode resource leasing"
In reply to: Keith Packard: "[PATCH 1/4] drm: Add new LEASE debug level"
Next in thread: Daniel Vetter: "Re: [PATCH 3/4] drm: Check mode object lease status in all master ioctl paths"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]