From 83d60795157c83389e6aaa0532d5e19afa976a24 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Sat, 6 Jun 2009 09:45:57 +0100 Subject: [PATCH] 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: [] warn_slowpath_fmt+0x57/0x6d [] ? get_pageblock_migratetype+0x18/0x1e [] ? free_hot_page+0xa/0xc [] ? __free_pages+0x16/0x1f [] ? shmem_truncate_range+0x63e/0x656 [] ? slob_page_alloc+0x146/0x1c8 [] warn_slowpath_null+0x10/0x15 [] kref_get+0x1b/0x21 [] i915_gem_object_move_to_active+0x1f/0x56 [] i915_add_request+0x156/0x19a [] i915_gem_object_flush_gpu_write_domain+0x28/0x3f [] i915_gem_object_unbind+0x4a/0x124 [] i915_gem_free_object+0x33/0x9b [] drm_gem_object_free+0x28/0x4a [] ? drm_gem_object_free+0x0/0x4a [] kref_put+0x38/0x41 [] drm_gem_object_unreference+0x11/0x13 [] drm_gem_object_handle_unreference+0x1e/0x21 [] drm_gem_object_release_handle+0xa/0xe [] idr_for_each+0x5f/0x98 [] ? drm_gem_object_release_handle+0x0/0xe [] drm_gem_release+0x22/0x34 [] drm_release+0x1e8/0x3c4 [] __fput+0xaf/0x146 [] fput+0x12/0x14 [] filp_close+0x48/0x52 [] put_files_struct+0x57/0x9b [] exit_files+0x1e/0x20 [] do_exit+0x16d/0x511 [] ? __schedule+0x3d4/0x3e5 [] ? handle_irq+0xd/0x69 [] do_group_exit+0x4d/0x73 [] sys_exit_group+0x13/0x17 [] sysenter_do_call+0x12/0x2b Signed-off-by: Chris Wilson Signed-off-by: Eric Anholt --- drivers/gpu/drm/i915/i915_gem.c | 38 +++++++++++++++++++++++---------- 1 file 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 @@ i915_gem_put_relocs_to_user(struct drm_i915_gem_exec_object *exec_list, 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, -- GitLab