On 5 October 2017 at 13:24, Dave Airlie airlied@gmail.com wrote:
On 5 October 2017 at 13:17, Dave Airlie airlied@gmail.com wrote:
drivers/gpu/drm/drm_ioctl.c | 4 + drivers/gpu/drm/drm_lease.c | 270 ++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_lease.h | 12 ++ include/uapi/drm/drm.h | 5 + include/uapi/drm/drm_mode.h | 64 +++++++++++ 5 files changed, 355 insertions(+)
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index a5a259964c7d..0a43e82d3f06 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -657,6 +657,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE, drm_syncobj_fd_to_handle_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),};
#define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c index a8bd4bdd2977..f233d8b488f2 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -23,6 +23,8 @@ #define drm_for_each_lessee(lessee, lessor) \ list_for_each_entry((lessee), &(lessor)->lessees, lessee_list)
+static uint64_t drm_lease_idr_object;
What is this for? ^^
ret = idr_alloc(&leases, &drm_lease_idr_object , object_id, object_id + 1, GFP_KERNEL);You can just pass NULL here.
Just read the comment, this smells a bit of black magic, I'll spend some time staring at it :-)
I think a comment in here is definitely warranted with what you expect from the idr interface here.
You seem to be storing the object ids in it from the user, and failing if you get a duplicate, then later dequeuing the idr.
I'm not sure this an intended use of idr :-), my brain is saying a bitmap would work just as well, or even why we want/need deduplication here. If userspace does something stupid does it matter?
Because otherwise you could just memdup_user the array of ids, and pass that in without the idr stuff.
Dave.