The patch below does not apply to the 3.12-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

>From 9ae9ab522094406915c27dc729d3ecef7b44af72 Mon Sep 17 00:00:00 2001
From: Chris Wilson <[email protected]>
Date: Wed, 4 Dec 2013 09:52:58 +0000
Subject: [PATCH] drm/i915: Prevent double unref following alloc failure during
 execbuffer

Whilst looking up the objects required for an execbuffer, an untimely
allocation failure in creating the vma results in the object being
unreferenced from two lists. The ownership during the lookup is meant to
be moved from the list of objects being looked to the vma, and this
double unreference upon error results in a use-after-free.

Fixes regression from
commit 27173f1f95db5e74ceb35fe9a2f2f348ea11bac9
Author: Ben Widawsky <[email protected]>
Date:   Wed Aug 14 11:38:36 2013 +0200

    drm/i915: Convert execbuf code to use vmas

Based on the fix by Ben Widawsky.

Signed-off-by: Chris Wilson <[email protected]>
Cc: Ben Widawsky <[email protected]>
Cc: [email protected]
[danvet: Bikeshed the crucial comment above the ownership transfer as
discussed on irc.]
Signed-off-by: Daniel Vetter <[email protected]>

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index b7e787fb4649..a3ba9a8cd687 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -93,7 +93,7 @@ eb_lookup_vmas(struct eb_vmas *eb,
 {
        struct drm_i915_gem_object *obj;
        struct list_head objects;
-       int i, ret = 0;
+       int i, ret;
 
        INIT_LIST_HEAD(&objects);
        spin_lock(&file->table_lock);
@@ -106,7 +106,7 @@ eb_lookup_vmas(struct eb_vmas *eb,
                        DRM_DEBUG("Invalid object handle %d at index %d\n",
                                   exec[i].handle, i);
                        ret = -ENOENT;
-                       goto out;
+                       goto err;
                }
 
                if (!list_empty(&obj->obj_exec_link)) {
@@ -114,7 +114,7 @@ eb_lookup_vmas(struct eb_vmas *eb,
                        DRM_DEBUG("Object %p [handle %d, index %d] appears more 
than once in object list\n",
                                   obj, exec[i].handle, i);
                        ret = -EINVAL;
-                       goto out;
+                       goto err;
                }
 
                drm_gem_object_reference(&obj->base);
@@ -123,9 +123,13 @@ eb_lookup_vmas(struct eb_vmas *eb,
        spin_unlock(&file->table_lock);
 
        i = 0;
-       list_for_each_entry(obj, &objects, obj_exec_link) {
+       while (!list_empty(&objects)) {
                struct i915_vma *vma;
 
+               obj = list_first_entry(&objects,
+                                      struct drm_i915_gem_object,
+                                      obj_exec_link);
+
                /*
                 * NOTE: We can leak any vmas created here when something fails
                 * later on. But that's no issue since vma_unbind can deal with
@@ -138,10 +142,12 @@ eb_lookup_vmas(struct eb_vmas *eb,
                if (IS_ERR(vma)) {
                        DRM_DEBUG("Failed to lookup VMA\n");
                        ret = PTR_ERR(vma);
-                       goto out;
+                       goto err;
                }
 
+               /* Transfer ownership from the objects list to the vmas list. */
                list_add_tail(&vma->exec_list, &eb->vmas);
+               list_del_init(&obj->obj_exec_link);
 
                vma->exec_entry = &exec[i];
                if (eb->and < 0) {
@@ -155,16 +161,22 @@ eb_lookup_vmas(struct eb_vmas *eb,
                ++i;
        }
 
+       return 0;
+
 
-out:
+err:
        while (!list_empty(&objects)) {
                obj = list_first_entry(&objects,
                                       struct drm_i915_gem_object,
                                       obj_exec_link);
                list_del_init(&obj->obj_exec_link);
-               if (ret)
-                       drm_gem_object_unreference(&obj->base);
+               drm_gem_object_unreference(&obj->base);
        }
+       /*
+        * Objects already transfered to the vmas list will be unreferenced by
+        * eb_destroy.
+        */
+
        return ret;
 }
 

--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to