Skip to content

Commit

Permalink
drm: Drop modeset_lock_all from the getproperty ioctl
Browse files Browse the repository at this point in the history
Properties, i.e. the struct drm_property specifying the type and value
range of a property, not the instantiation on a given object, are
invariant over the lifetime of a driver.

Hence no locking at all is needed, we can just remove it.

While at it give the function some love and simplify it, to get it
under the 80 char limit:
- Straighten the loops to reduce the nesting.
- use u64_to_user_ptr casting helper
- use put_user for fixed u64 copies.

Note there's a small behavioural change in that we now copy parts of
the values to userspace if the arrays are a bit too small. Since
userspace will immediately retry anyway, this doesn't matter.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170403083304.9083-7-daniel.vetter@ffwll.ch
  • Loading branch information
Daniel Vetter committed Apr 5, 2017
1 parent c2d8556 commit eb8eb02
Showing 1 changed file with 29 additions and 43 deletions.
72 changes: 29 additions & 43 deletions drivers/gpu/drm/drm_property.c
Original file line number Diff line number Diff line change
Expand Up @@ -442,64 +442,51 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
struct drm_property *property;
int enum_count = 0;
int value_count = 0;
int ret = 0, i;
int copied;
int i, copied;
struct drm_property_enum *prop_enum;
struct drm_mode_property_enum __user *enum_ptr;
uint64_t __user *values_ptr;

if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EINVAL;

drm_modeset_lock_all(dev);
property = drm_property_find(dev, out_resp->prop_id);
if (!property) {
ret = -ENOENT;
goto done;
}

if (drm_property_type_is(property, DRM_MODE_PROP_ENUM) ||
drm_property_type_is(property, DRM_MODE_PROP_BITMASK)) {
list_for_each_entry(prop_enum, &property->enum_list, head)
enum_count++;
}

value_count = property->num_values;
if (!property)
return -ENOENT;

strncpy(out_resp->name, property->name, DRM_PROP_NAME_LEN);
out_resp->name[DRM_PROP_NAME_LEN-1] = 0;
out_resp->flags = property->flags;

if ((out_resp->count_values >= value_count) && value_count) {
values_ptr = (uint64_t __user *)(unsigned long)out_resp->values_ptr;
for (i = 0; i < value_count; i++) {
if (copy_to_user(values_ptr + i, &property->values[i], sizeof(uint64_t))) {
ret = -EFAULT;
goto done;
}
value_count = property->num_values;
values_ptr = u64_to_user_ptr(out_resp->values_ptr);

for (i = 0; i < value_count; i++) {
if (i < out_resp->count_values &&
put_user(property->values[i], values_ptr + i)) {
return -EFAULT;
}
}
out_resp->count_values = value_count;

copied = 0;
enum_ptr = u64_to_user_ptr(out_resp->enum_blob_ptr);

if (drm_property_type_is(property, DRM_MODE_PROP_ENUM) ||
drm_property_type_is(property, DRM_MODE_PROP_BITMASK)) {
if ((out_resp->count_enum_blobs >= enum_count) && enum_count) {
copied = 0;
enum_ptr = (struct drm_mode_property_enum __user *)(unsigned long)out_resp->enum_blob_ptr;
list_for_each_entry(prop_enum, &property->enum_list, head) {

if (copy_to_user(&enum_ptr[copied].value, &prop_enum->value, sizeof(uint64_t))) {
ret = -EFAULT;
goto done;
}

if (copy_to_user(&enum_ptr[copied].name,
&prop_enum->name, DRM_PROP_NAME_LEN)) {
ret = -EFAULT;
goto done;
}
copied++;
}
drm_property_type_is(property, DRM_MODE_PROP_BITMASK)) {
list_for_each_entry(prop_enum, &property->enum_list, head) {
enum_count++;
if (out_resp->count_enum_blobs <= enum_count)
continue;

if (copy_to_user(&enum_ptr[copied].value,
&prop_enum->value, sizeof(uint64_t)))
return -EFAULT;

if (copy_to_user(&enum_ptr[copied].name,
&prop_enum->name, DRM_PROP_NAME_LEN))
return -EFAULT;
copied++;
}
out_resp->count_enum_blobs = enum_count;
}
Expand All @@ -514,9 +501,8 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
*/
if (drm_property_type_is(property, DRM_MODE_PROP_BLOB))
out_resp->count_enum_blobs = 0;
done:
drm_modeset_unlock_all(dev);
return ret;

return 0;
}

static void drm_property_free_blob(struct kref *kref)
Expand Down

0 comments on commit eb8eb02

Please sign in to comment.