In vma destruction, the following race may occur: Thread 1: Thread 2: i915_vma_destroy(); ... list_del_init(vma->vm_link); ... mutex_unlock(vma->vm->mutex); __i915_vm_release(); release_references(); And in release_reference() we dereference vma->vm to get to the vm gt pointer, leading to a use-after free. However, __i915_vm_release() grabs the vm->mutex so the vm won't be destroyed before vma->vm->mutex is released, so extract the gt pointer under the vm->mutex to avoid the vma->vm dereference in release_references(). v2: Fix a typo in the commit message (Andi Shyti) Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5944 Fixes: e1a7ab4fca0c ("drm/i915: Remove the vm open count")
Unified · Split
| @@ -1637,10 +1637,10 @@ static void force_unbind(struct i915_vma *vma) · 1636 unmodified lines | |||
| 1637 | GEM_BUG_ON(drm_mm_node_allocated(&vma->node)); | 1637 | GEM_BUG_ON(drm_mm_node_allocated(&vma->node)); |
| 1638 | } | 1638 | } |
| 1639 | 1639 | ||
| 1640 | static void release_references(struct i915_vma *vma, bool vm_ddestroy) | 1640 | static void release_references(struct i915_vma *vma, struct intel_gt *gt, |
| 1641 | bool vm_ddestroy) | ||
| 1641 | { | 1642 | { |
| 1642 | struct drm_i915_gem_object *obj = vma->obj; | 1643 | struct drm_i915_gem_object *obj = vma->obj; |
| 1643 | struct intel_gt *gt = vma->vm->gt; | ||
| 1644 | 1644 | ||
| 1645 | GEM_BUG_ON(i915_vma_is_active(vma)); | 1645 | GEM_BUG_ON(i915_vma_is_active(vma)); |
| 1646 | 1646 | ||
| @@ -1695,11 +1695,12 @@ void i915_vma_destroy_locked(struct i915_vma *vma) · 48 unmodified lines | |||
| 1695 | 1695 | ||
| 1696 | force_unbind(vma); | 1696 | force_unbind(vma); |
| 1697 | list_del_init(&vma->vm_link); | 1697 | list_del_init(&vma->vm_link); |
| 1698 | release_references(vma, false); | 1698 | release_references(vma, vma->vm->gt, false); |
| 1699 | } | 1699 | } |
| 1700 | 1700 | ||
| 1701 | void i915_vma_destroy(struct i915_vma *vma) | 1701 | void i915_vma_destroy(struct i915_vma *vma) |
| 1702 | { | 1702 | { |
| 1703 | struct intel_gt *gt; | ||
| 1703 | bool vm_ddestroy; | 1704 | bool vm_ddestroy; |
| 1704 | 1705 | ||
| 1705 | mutex_lock(&vma->vm->mutex); | 1706 | mutex_lock(&vma->vm->mutex); |
| @@ -1707,8 +1708,11 @@ void i915_vma_destroy(struct i915_vma *vma) · 1 unmodified line | |||
| 1707 | list_del_init(&vma->vm_link); | 1708 | list_del_init(&vma->vm_link); |
| 1708 | vm_ddestroy = vma->vm_ddestroy; | 1709 | vm_ddestroy = vma->vm_ddestroy; |
| 1709 | vma->vm_ddestroy = false; | 1710 | vma->vm_ddestroy = false; |
| 1711 | |||
| 1712 | /* vma->vm may be freed when releasing vma->vm->mutex. */ | ||
| 1713 | gt = vma->vm->gt; | ||
| 1710 | mutex_unlock(&vma->vm->mutex); | 1714 | mutex_unlock(&vma->vm->mutex); |
| 1711 | release_references(vma, vm_ddestroy); | 1715 | release_references(vma, gt, vm_ddestroy); |
| 1712 | } | 1716 | } |
| 1713 | 1717 | ||
| 1714 | void i915_vma_parked(struct intel_gt *gt) | 1718 | void i915_vma_parked(struct intel_gt *gt) |