From 95d4c8702dbd0bbc06291105c3fbfe1927b13f2d Mon Sep 17 00:00:00 2001 From: Florian Mickler Date: Mon, 6 Apr 2009 21:35:33 +0200 Subject: [PATCH] Fix: use of uninitialized var i915_gem_put_relocs_to_user returned an uninitialized value which got returned to userspace. This caused libdrm in my setup to never get out of a do{}while() loop retrying i915_gem_execbuffer. result was hanging X, overheating of cpu and 2-3gb of logfile-spam. This patch adresses the issue by 1. initializing vars in this file where necessary 2. correcting wrongly interpreted return values of copy_[from/to]_user Nr. 2 helps libdrm from getting out of its loop and Nr. 1 helps i915_gem_execbuffer to not think there was an error. Signed-off-by: Florian Mickler --- drivers/gpu/drm/i915/i915_gem.c | 63 +++++++++++++++++++++++++------------- 1 files changed, 41 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 1449b45..99c01f5 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -151,6 +151,7 @@ fast_shmem_read(struct page **pages, ret = __copy_to_user_inatomic(data, vaddr + page_offset, length); kunmap_atomic(vaddr, KM_USER0); + /* return a number of bytes */ return ret; } @@ -2976,7 +2977,7 @@ i915_gem_get_relocs_from_user(struct drm_i915_gem_exec_object *exec_list, struct drm_i915_gem_relocation_entry **relocs) { uint32_t reloc_count = 0, reloc_index = 0, i; - int ret; + int ret = 0; *relocs = NULL; for (i = 0; i < buffer_count; i++) { @@ -3002,13 +3003,14 @@ i915_gem_get_relocs_from_user(struct drm_i915_gem_exec_object *exec_list, drm_free(*relocs, reloc_count * sizeof(**relocs), DRM_MEM_DRIVER); *relocs = NULL; - return ret; + return -EFAULT; } reloc_index += exec_list[i].relocation_count; } - return ret; + /* success */ + return 0; } static int @@ -3017,14 +3019,14 @@ i915_gem_put_relocs_to_user(struct drm_i915_gem_exec_object *exec_list, struct drm_i915_gem_relocation_entry *relocs) { uint32_t reloc_count = 0, i; - int ret; + int ret = 0; for (i = 0; i < buffer_count; i++) { struct drm_i915_gem_relocation_entry __user *user_relocs; user_relocs = (void __user *)(uintptr_t)exec_list[i].relocs_ptr; - if (ret == 0) { + if ( ret == 0) { ret = copy_to_user(user_relocs, &relocs[reloc_count], exec_list[i].relocation_count * @@ -3036,6 +3038,12 @@ i915_gem_put_relocs_to_user(struct drm_i915_gem_exec_object *exec_list, drm_free(relocs, reloc_count * sizeof(*relocs), DRM_MEM_DRIVER); + /* copy_to_user returns a number of bytes */ + if (ret != 0) { + ret = -EFAULT; + } + + return ret; } @@ -3052,11 +3060,14 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, struct drm_i915_gem_object *obj_priv; struct drm_clip_rect *cliprects = NULL; struct drm_i915_gem_relocation_entry *relocs; - int ret, ret2, i, pinned = 0; + int ret, error, i, pinned; uint64_t exec_offset; uint32_t seqno, flush_domains, reloc_index; int pin_tries; + error = 0; + pinned = 0; + #if WATCH_EXEC DRM_INFO("buffers_ptr %d buffer_count %d len %08x\n", (int) args->buffers_ptr, args->buffer_count, args->batch_len); @@ -3075,7 +3086,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, DRM_ERROR("Failed to allocate exec or object list " "for %d buffers\n", args->buffer_count); - ret = -ENOMEM; + error = -ENOMEM; goto pre_mutex_err; } ret = copy_from_user(exec_list, @@ -3085,6 +3096,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, if (ret != 0) { DRM_ERROR("copy %d exec entries failed %d\n", args->buffer_count, ret); + error = ret; goto pre_mutex_err; } @@ -3101,15 +3113,17 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, if (ret != 0) { DRM_ERROR("copy %d cliprects failed: %d\n", args->num_cliprects, ret); + error = ret; goto pre_mutex_err; } } ret = i915_gem_get_relocs_from_user(exec_list, args->buffer_count, &relocs); - if (ret != 0) + if (ret != 0) { + error = ret; goto pre_mutex_err; - + } mutex_lock(&dev->struct_mutex); i915_verify_inactive(dev, __FILE__, __LINE__); @@ -3117,14 +3131,14 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, if (dev_priv->mm.wedged) { DRM_ERROR("Execbuf while wedged\n"); mutex_unlock(&dev->struct_mutex); - ret = -EIO; + error = -EIO; goto pre_mutex_err; } if (dev_priv->mm.suspended) { DRM_ERROR("Execbuf while VT-switched.\n"); mutex_unlock(&dev->struct_mutex); - ret = -EBUSY; + error = -EBUSY; goto pre_mutex_err; } @@ -3135,7 +3149,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, if (object_list[i] == NULL) { DRM_ERROR("Invalid object handle %d at index %d\n", exec_list[i].handle, i); - ret = -EBADF; + error = -EBADF; goto err; } @@ -3143,7 +3157,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, if (obj_priv->in_execbuffer) { DRM_ERROR("Object %p appears more than once in object list\n", object_list[i]); - ret = -EBADF; + error = -EBADF; goto err; } obj_priv->in_execbuffer = true; @@ -3174,6 +3188,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, if (ret != -ENOMEM || pin_tries >= 1) { if (ret != -ERESTARTSYS) DRM_ERROR("Failed to pin buffers %d\n", ret); + error = ret; goto err; } @@ -3184,8 +3199,10 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, /* evict everyone we can from the aperture */ ret = i915_gem_evict_everything(dev); - if (ret) + if (ret){ + error = ret; goto err; + } } /* Set the pending read domains for the batch buffer to COMMAND */ @@ -3253,6 +3270,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, ret = i915_dispatch_gem_execbuffer(dev, args, cliprects, exec_offset); if (ret) { DRM_ERROR("dispatch failed %d\n", ret); + error = ret; goto err; } @@ -3308,10 +3326,12 @@ err: (uintptr_t) args->buffers_ptr, exec_list, sizeof(*exec_list) * args->buffer_count); - if (ret) + if (ret) { + error = -EFAULT; DRM_ERROR("failed to copy %d exec entries " "back to user (%d)\n", args->buffer_count, ret); + } } /* Copy the updated relocations out regardless of current error @@ -3319,13 +3339,12 @@ err: * time userland calls execbuf, it would do so with presumed offset * state that didn't match the actual object state. */ - ret2 = i915_gem_put_relocs_to_user(exec_list, args->buffer_count, + ret = i915_gem_put_relocs_to_user(exec_list, args->buffer_count, relocs); - if (ret2 != 0) { - DRM_ERROR("Failed to copy relocations back out: %d\n", ret2); - - if (ret == 0) - ret = ret2; + if (ret != 0) { + DRM_ERROR("Failed to copy relocations back out\n"); + if (error == 0) + error = ret; } pre_mutex_err: @@ -3336,7 +3355,7 @@ pre_mutex_err: drm_free(cliprects, sizeof(*cliprects) * args->num_cliprects, DRM_MEM_DRIVER); - return ret; + return error; } int -- 1.6.2