From 2bb4629add2e648f1822872cce72f0fe163ce605 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Fri, 22 Feb 2013 16:12:51 +0200 Subject: drm/i915: Add to_user_ptr() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit to_user_ptr() simply casts a pointer passed as u64 from user space to void __user * correctly. Using this lets us get rid of all the tiresome casts. The idea came from Chris Wilson . Signed-off-by: Ville Syrjälä Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 2f2daebd0eef..934396c5f048 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -305,7 +305,7 @@ i915_gem_execbuffer_relocate_object(struct drm_i915_gem_object *obj, struct drm_i915_gem_exec_object2 *entry = obj->exec_entry; int remain, ret; - user_relocs = (void __user *)(uintptr_t)entry->relocs_ptr; + user_relocs = to_user_ptr(entry->relocs_ptr); remain = entry->relocation_count; while (remain) { @@ -618,7 +618,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev, u64 invalid_offset = (u64)-1; int j; - user_relocs = (void __user *)(uintptr_t)exec[i].relocs_ptr; + user_relocs = to_user_ptr(exec[i].relocs_ptr); if (copy_from_user(reloc+total, user_relocs, exec[i].relocation_count * sizeof(*reloc))) { @@ -734,7 +734,7 @@ validate_exec_list(struct drm_i915_gem_exec_object2 *exec, int i; for (i = 0; i < count; i++) { - char __user *ptr = (char __user *)(uintptr_t)exec[i].relocs_ptr; + char __user *ptr = to_user_ptr(exec[i].relocs_ptr); int length; /* limited by fault_in_pages_readable() */ if (exec[i].flags & __EXEC_OBJECT_UNKNOWN_FLAGS) @@ -944,9 +944,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, } if (copy_from_user(cliprects, - (struct drm_clip_rect __user *)(uintptr_t) - args->cliprects_ptr, - sizeof(*cliprects)*args->num_cliprects)) { + to_user_ptr(args->cliprects_ptr), + sizeof(*cliprects)*args->num_cliprects)) { ret = -EFAULT; goto pre_mutex_err; } @@ -1110,7 +1109,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, return -ENOMEM; } ret = copy_from_user(exec_list, - (void __user *)(uintptr_t)args->buffers_ptr, + to_user_ptr(args->buffers_ptr), sizeof(*exec_list) * args->buffer_count); if (ret != 0) { DRM_DEBUG("copy %d exec entries failed %d\n", @@ -1149,7 +1148,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, for (i = 0; i < args->buffer_count; i++) exec_list[i].offset = exec2_list[i].offset; /* ... and back out to userspace */ - ret = copy_to_user((void __user *)(uintptr_t)args->buffers_ptr, + ret = copy_to_user(to_user_ptr(args->buffers_ptr), exec_list, sizeof(*exec_list) * args->buffer_count); if (ret) { @@ -1190,8 +1189,7 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data, return -ENOMEM; } ret = copy_from_user(exec2_list, - (struct drm_i915_relocation_entry __user *) - (uintptr_t) args->buffers_ptr, + to_user_ptr(args->buffers_ptr), sizeof(*exec2_list) * args->buffer_count); if (ret != 0) { DRM_DEBUG("copy %d exec entries failed %d\n", @@ -1203,7 +1201,7 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data, ret = i915_gem_do_execbuffer(dev, data, file, args, exec2_list); if (!ret) { /* Copy the new buffer offsets back to the user's exec list. */ - ret = copy_to_user((void __user *)(uintptr_t)args->buffers_ptr, + ret = copy_to_user(to_user_ptr(args->buffers_ptr), exec2_list, sizeof(*exec2_list) * args->buffer_count); if (ret) { -- cgit v1.2.3 From 3058753583c6a641bac188011b4d777adec916c9 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 11 Mar 2013 14:37:35 -0700 Subject: drm/i915: clarify reasoning for the access_ok call This clarifies the comment above the access_ok check so a missing VERIFY_READ doesn't alarm anyone. v2: - rewrote comment, thanks to Chris Wilson Signed-off-by: Kees Cook Cc: Daniel Vetter [danvet: add patch history log to commit message.] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 934396c5f048..ea963c32772d 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -747,7 +747,11 @@ validate_exec_list(struct drm_i915_gem_exec_object2 *exec, length = exec[i].relocation_count * sizeof(struct drm_i915_gem_relocation_entry); - /* we may also need to update the presumed offsets */ + /* + * We must check that the entire relocation array is safe + * to read, but since we may need to update the presumed + * offsets during execution, check for full write access. + */ if (!access_ok(VERIFY_WRITE, ptr, length)) return -EFAULT; -- cgit v1.2.3 From cf144969d54da1476e2b80dd632952a2f93c6b34 Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Wed, 13 Mar 2013 17:21:05 -0700 Subject: drm/i915: Remove unused file arg from execbuf Signed-off-by: Ben Widawsky Acked-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index ea963c32772d..87948a467641 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -475,7 +475,6 @@ i915_gem_execbuffer_unreserve_object(struct drm_i915_gem_object *obj) static int i915_gem_execbuffer_reserve(struct intel_ring_buffer *ring, - struct drm_file *file, struct list_head *objects, bool *need_relocs) { @@ -663,7 +662,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev, goto err; need_relocs = (args->flags & I915_EXEC_NO_RELOC) == 0; - ret = i915_gem_execbuffer_reserve(ring, file, &eb->objects, &need_relocs); + ret = i915_gem_execbuffer_reserve(ring, &eb->objects, &need_relocs); if (ret) goto err; @@ -984,7 +983,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, /* Move the objects en-masse into the GTT, evicting if necessary. */ need_relocs = (args->flags & I915_EXEC_NO_RELOC) == 0; - ret = i915_gem_execbuffer_reserve(ring, file, &eb->objects, &need_relocs); + ret = i915_gem_execbuffer_reserve(ring, &eb->objects, &need_relocs); if (ret) goto err; -- cgit v1.2.3 From 41fda596826c5db206f67ce5d639f65a19924a99 Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Wed, 13 Mar 2013 17:21:06 -0700 Subject: drm/i915: Remove unneeded dev argument Signed-off-by: Ben Widawsky Acked-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 87948a467641..983083976dd8 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -359,8 +359,7 @@ i915_gem_execbuffer_relocate_object_slow(struct drm_i915_gem_object *obj, } static int -i915_gem_execbuffer_relocate(struct drm_device *dev, - struct eb_objects *eb) +i915_gem_execbuffer_relocate(struct eb_objects *eb) { struct drm_i915_gem_object *obj; int ret = 0; @@ -989,7 +988,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, /* The objects are in their final locations, apply the relocations. */ if (need_relocs) - ret = i915_gem_execbuffer_relocate(dev, eb); + ret = i915_gem_execbuffer_relocate(eb); if (ret) { if (ret == -EFAULT) { ret = i915_gem_execbuffer_relocate_slow(dev, args, file, ring, -- cgit v1.2.3