summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorChris Wilson <chris@chris-wilson.co.uk>2009-06-06 12:45:57 +0400
committerEric Anholt <eric@anholt.net>2009-06-10 00:52:57 +0400
commit83d60795157c83389e6aaa0532d5e19afa976a24 (patch)
tree73ccda19411b2b6228a54c0869cfa114da617611
parentfa0864b26b4bfa1dd4bb78eeffbc1f398cb56425 (diff)
downloadlinux-83d60795157c83389e6aaa0532d5e19afa976a24.tar.xz
drm/i915: Sanity check execbuffer arguments before touching state.
By sending a broken execbuffer (its length was not suitably aligned) I triggered an operation upon a freed object. The invalid alignment was discovered after updating the write_domain on the object but before the object was placed on the active queue. So during the unwind process following the error, the now freed object attempts to flush its non-existent, but outstanding, GPU writes causing this use-after-free. [drm:i915_dispatch_gem_execbuffer] *ERROR* alignment [drm:i915_gem_execbuffer] *ERROR* dispatch failed -22 WARNING: at lib/kref.c:43 warn_slowpath_null+0x10/0x15() Modules linked in: Pid: 4552, comm: lt-csi-drm Not tainted 2.6.30-rc6 #423 Call Trace: [<c0119ef3>] warn_slowpath_fmt+0x57/0x6d [<c014de24>] ? get_pageblock_migratetype+0x18/0x1e [<c014e8fd>] ? free_hot_page+0xa/0xc [<c014e915>] ? __free_pages+0x16/0x1f [<c0153ebf>] ? shmem_truncate_range+0x63e/0x656 [<c015fb2f>] ? slob_page_alloc+0x146/0x1c8 [<c0119f19>] warn_slowpath_null+0x10/0x15 [<c01f55f2>] kref_get+0x1b/0x21 [<c02605db>] i915_gem_object_move_to_active+0x1f/0x56 [<c0261302>] i915_add_request+0x156/0x19a [<c026136e>] i915_gem_object_flush_gpu_write_domain+0x28/0x3f [<c0261eca>] i915_gem_object_unbind+0x4a/0x124 [<c0261fd7>] i915_gem_free_object+0x33/0x9b [<c0250d6b>] drm_gem_object_free+0x28/0x4a [<c0250d43>] ? drm_gem_object_free+0x0/0x4a [<c01f55ce>] kref_put+0x38/0x41 [<c0250cbf>] drm_gem_object_unreference+0x11/0x13 [<c0250d06>] drm_gem_object_handle_unreference+0x1e/0x21 [<c0250d13>] drm_gem_object_release_handle+0xa/0xe [<c01f3e6b>] idr_for_each+0x5f/0x98 [<c0250d09>] ? drm_gem_object_release_handle+0x0/0xe [<c0250daf>] drm_gem_release+0x22/0x34 [<c025046f>] drm_release+0x1e8/0x3c4 [<c0162d25>] __fput+0xaf/0x146 [<c0162dce>] fput+0x12/0x14 [<c01605ef>] filp_close+0x48/0x52 [<c011b182>] put_files_struct+0x57/0x9b [<c011b1e4>] exit_files+0x1e/0x20 [<c011c6b6>] do_exit+0x16d/0x511 [<c03704ab>] ? __schedule+0x3d4/0x3e5 [<c0103f0d>] ? handle_irq+0xd/0x69 [<c011caa7>] do_group_exit+0x4d/0x73 [<c011cae0>] sys_exit_group+0x13/0x17 [<c010268c>] sysenter_do_call+0x12/0x2b Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Eric Anholt <eric@anholt.net>
-rw-r--r--drivers/gpu/drm/i915/i915_gem.c38
1 files changed, 27 insertions, 11 deletions
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 38e0f8301a14..ac22668b239a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3050,20 +3050,12 @@ i915_dispatch_gem_execbuffer(struct drm_device *dev,
drm_i915_private_t *dev_priv = dev->dev_private;
int nbox = exec->num_cliprects;
int i = 0, count;
- uint32_t exec_start, exec_len;
+ uint32_t exec_start, exec_len;
RING_LOCALS;
exec_start = (uint32_t) exec_offset + exec->batch_start_offset;
exec_len = (uint32_t) exec->batch_len;
- if ((exec_start | exec_len) & 0x7) {
- DRM_ERROR("alignment\n");
- return -EINVAL;
- }
-
- if (!exec_start)
- return -EINVAL;
-
count = nbox ? nbox : 1;
for (i = 0; i < count; i++) {
@@ -3211,6 +3203,24 @@ err:
return ret;
}
+static int
+i915_gem_check_execbuffer (struct drm_i915_gem_execbuffer *exec,
+ uint64_t exec_offset)
+{
+ uint32_t exec_start, exec_len;
+
+ exec_start = (uint32_t) exec_offset + exec->batch_start_offset;
+ exec_len = (uint32_t) exec->batch_len;
+
+ if ((exec_start | exec_len) & 0x7)
+ return -EINVAL;
+
+ if (!exec_start)
+ return -EINVAL;
+
+ return 0;
+}
+
int
i915_gem_execbuffer(struct drm_device *dev, void *data,
struct drm_file *file_priv)
@@ -3362,6 +3372,14 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
batch_obj->pending_read_domains = I915_GEM_DOMAIN_COMMAND;
batch_obj->pending_write_domain = 0;
+ /* Sanity check the batch buffer, prior to moving objects */
+ exec_offset = exec_list[args->buffer_count - 1].offset;
+ ret = i915_gem_check_execbuffer (args, exec_offset);
+ if (ret != 0) {
+ DRM_ERROR("execbuf with invalid offset/length\n");
+ goto err;
+ }
+
i915_verify_inactive(dev, __FILE__, __LINE__);
/* Zero the global flush/invalidate flags. These
@@ -3410,8 +3428,6 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
}
#endif
- exec_offset = exec_list[args->buffer_count - 1].offset;
-
#if WATCH_EXEC
i915_gem_dump_object(batch_obj,
args->batch_len,