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 <ben@bwidawsk.net>
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 <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: stable@vger.kernel.org
[danvet: Bikeshed the crucial comment above the ownership transfer as
discussed on irc.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
{
struct drm_i915_gem_object *obj;
struct list_head objects;
{
struct drm_i915_gem_object *obj;
struct list_head objects;
INIT_LIST_HEAD(&objects);
spin_lock(&file->table_lock);
INIT_LIST_HEAD(&objects);
spin_lock(&file->table_lock);
DRM_DEBUG("Invalid object handle %d at index %d\n",
exec[i].handle, i);
ret = -ENOENT;
DRM_DEBUG("Invalid object handle %d at index %d\n",
exec[i].handle, i);
ret = -ENOENT;
}
if (!list_empty(&obj->obj_exec_link)) {
}
if (!list_empty(&obj->obj_exec_link)) {
DRM_DEBUG("Object %p [handle %d, index %d] appears more than once in object list\n",
obj, exec[i].handle, i);
ret = -EINVAL;
DRM_DEBUG("Object %p [handle %d, index %d] appears more than once in object list\n",
obj, exec[i].handle, i);
ret = -EINVAL;
}
drm_gem_object_reference(&obj->base);
}
drm_gem_object_reference(&obj->base);
spin_unlock(&file->table_lock);
i = 0;
spin_unlock(&file->table_lock);
i = 0;
- list_for_each_entry(obj, &objects, obj_exec_link) {
+ while (!list_empty(&objects)) {
+ 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
/*
* NOTE: We can leak any vmas created here when something fails
* later on. But that's no issue since vma_unbind can deal with
if (IS_ERR(vma)) {
DRM_DEBUG("Failed to lookup VMA\n");
ret = PTR_ERR(vma);
if (IS_ERR(vma)) {
DRM_DEBUG("Failed to lookup VMA\n");
ret = PTR_ERR(vma);
+ /* Transfer ownership from the objects list to the vmas list. */
list_add_tail(&vma->exec_list, &eb->vmas);
list_add_tail(&vma->exec_list, &eb->vmas);
+ list_del_init(&obj->obj_exec_link);
vma->exec_entry = &exec[i];
if (eb->and < 0) {
vma->exec_entry = &exec[i];
if (eb->and < 0) {
while (!list_empty(&objects)) {
obj = list_first_entry(&objects,
struct drm_i915_gem_object,
obj_exec_link);
list_del_init(&obj->obj_exec_link);
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.
+ */
+