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>

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 38e0f8301a14e45e19fd826b069d3cdc65df73c9..ac22668b239a5b46bcedd089029e1668f7fbf876 100644 (file)
--- 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,