Skip to content

Commit

Permalink
drm/i915: Sanity check execbuffer arguments before touching state.
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Chris Wilson authored and Eric Anholt committed Jun 9, 2009
1 parent fa0864b commit 83d6079
Showing 1 changed file with 27 additions and 11 deletions.
38 changes: 27 additions & 11 deletions drivers/gpu/drm/i915/i915_gem.c
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 83d6079

Please sign in to comment.