[Intel-gfx] [PATCH 2/3] drm/i915: Pass the right flags to i915_vma_move_to_active()

Tue Jun 20 15:47:06 UTC 2017

On 20/06/2017 13:43, Chris Wilson wrote:
> i915_vma_move_to_active() takes the execobject flags and not a boolean!
> Instead of passing EXEC_OBJECT_WRITE we passed true [i.e.
> EXEC_OBJECT_NEEDS_FENCE] causing us to start tracking the
> vma->last_fence access and since we forgot to clear that on unbinding,
> we caused a use-after-free.
> 
> [  321.263854] BUG: KASAN: use-after-free in i915_gem_request_retire+0x1728/0x1740 [i915]
> [  321.264001] Read of size 8 at addr ffff880100fc67d8 by task gem_exec_reloc/2868
> 
> [  321.264181] CPU: 0 PID: 2868 Comm: gem_exec_reloc Not tainted 4.12.0-rc6-CI-Custom_2759+ #1
> [  321.264195] Hardware name: GIGABYTE GB-BXBT-1900/MZBAYAB-00, BIOS F6 02/17/2015
> [  321.264208] Call Trace:
> [  321.264234]  dump_stack+0x67/0x99
> [  321.264260]  print_address_description+0x77/0x290
> [  321.264437]  ? i915_gem_request_retire+0x1728/0x1740 [i915]
> [  321.264459]  kasan_report+0x269/0x350
> [  321.264487]  __asan_report_load8_noabort+0x14/0x20
> [  321.264660]  i915_gem_request_retire+0x1728/0x1740 [i915]
> [  321.264841]  ? intel_ring_context_pin+0x131/0x690 [i915]
> [  321.265021]  i915_gem_request_alloc+0x2c6/0x1220 [i915]
> [  321.265044]  ? _raw_spin_unlock_irqrestore+0x3d/0x60
> [  321.265226]  i915_gem_do_execbuffer+0xac0/0x2a20 [i915]
> [  321.265250]  ? __lock_acquire+0xceb/0x5450
> [  321.265269]  ? entry_SYSCALL_64_fastpath+0x1c/0xb1
> [  321.265291]  ? kvmalloc_node+0x6b/0x80
> [  321.265310]  ? kvmalloc_node+0x6b/0x80
> [  321.265489]  ? eb_relocate_slow+0xbe0/0xbe0 [i915]
> [  321.265520]  ? ___slab_alloc.constprop.28+0x2ab/0x3d0
> [  321.265549]  ? debug_check_no_locks_freed+0x280/0x280
> [  321.265591]  ? __might_fault+0xc6/0x1b0
> [  321.265782]  i915_gem_execbuffer2+0x14a/0x3f0 [i915]
> [  321.265815]  drm_ioctl+0x4ba/0xaa0
> [  321.265986]  ? i915_gem_execbuffer+0xde0/0xde0 [i915]
> [  321.266017]  ? drm_getunique+0x270/0x270
> [  321.266068]  do_vfs_ioctl+0x17f/0xfa0
> [  321.266091]  ? __fget+0x1ba/0x330
> [  321.266112]  ? lock_acquire+0x390/0x390
> [  321.266133]  ? ioctl_preallocate+0x1d0/0x1d0
> [  321.266164]  ? __fget+0x1db/0x330
> [  321.266194]  ? __fget_light+0x79/0x1f0
> [  321.266219]  SyS_ioctl+0x3c/0x70
> [  321.266247]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> [  321.266265] RIP: 0033:0x7fcede207357
> [  321.266279] RSP: 002b:00007ffef0effe58 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> [  321.266307] RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007fcede207357
> [  321.266321] RDX: 00007ffef0effef0 RSI: 0000000040406469 RDI: 0000000000000004
> [  321.266335] RBP: ffffffff812097c6 R08: 0000000000000008 R09: 0000000000000000
> [  321.266349] R10: 0000000000000008 R11: 0000000000000246 R12: ffff880116bcff98
> [  321.266363] R13: ffffffff81cb7cb3 R14: ffff880116bcff70 R15: 0000000000000000
> [  321.266385]  ? __this_cpu_preempt_check+0x13/0x20
> [  321.266406]  ? trace_hardirqs_off_caller+0x1d6/0x2c0
> 
> [  321.266487] Allocated by task 2868:
> [  321.266568]  save_stack_trace+0x16/0x20
> [  321.266586]  kasan_kmalloc+0xee/0x180
> [  321.266602]  kasan_slab_alloc+0x12/0x20
> [  321.266620]  kmem_cache_alloc+0xc7/0x2e0
> [  321.266795]  i915_vma_instance+0x28c/0x1540 [i915]
> [  321.266964]  eb_lookup_vmas+0x5a7/0x2250 [i915]
> [  321.267130]  i915_gem_do_execbuffer+0x69a/0x2a20 [i915]
> [  321.267296]  i915_gem_execbuffer2+0x14a/0x3f0 [i915]
> [  321.267315]  drm_ioctl+0x4ba/0xaa0
> [  321.267333]  do_vfs_ioctl+0x17f/0xfa0
> [  321.267350]  SyS_ioctl+0x3c/0x70
> [  321.267369]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> 
> [  321.267428] Freed by task 177:
> [  321.267502]  save_stack_trace+0x16/0x20
> [  321.267521]  kasan_slab_free+0xad/0x180
> [  321.267539]  kmem_cache_free+0xc5/0x340
> [  321.267710]  i915_vma_unbind+0x666/0x10a0 [i915]
> [  321.267880]  i915_vma_close+0x23a/0x2f0 [i915]
> [  321.268048]  __i915_gem_free_objects+0x17d/0xc70 [i915]
> [  321.268215]  __i915_gem_free_work+0x49/0x70 [i915]
> [  321.268234]  process_one_work+0x66f/0x1410
> [  321.268252]  worker_thread+0xe1/0xe90
> [  321.268269]  kthread+0x304/0x410
> [  321.268285]  ret_from_fork+0x27/0x40
> 
> [  321.268346] The buggy address belongs to the object at ffff880100fc6640
>                  which belongs to the cache i915_vma of size 656
> [  321.268550] The buggy address is located 408 bytes inside of
>                  656-byte region [ffff880100fc6640, ffff880100fc68d0)
> [  321.268741] The buggy address belongs to the page:
> [  321.268837] page:ffffea000403f000 count:1 mapcount:0 mapping:          (null) index:0xffff880100fc5980 compound_mapcount: 0
> [  321.269045] flags: 0x8000000000008100(slab|head)
> [  321.269147] raw: 8000000000008100 0000000000000000 ffff880100fc5980 00000001001e001d
> [  321.269312] raw: ffffea0004038e20 ffff880116b46240 ffff88011646c640 0000000000000000
> [  321.269484] page dumped because: kasan: bad access detected
> 
> [  321.269665] Memory state around the buggy address:
> [  321.269778]  ffff880100fc6680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [  321.269949]  ffff880100fc6700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [  321.270115] >ffff880100fc6780: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [  321.270279]                                                     ^
> [  321.270410]  ffff880100fc6800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [  321.270576]  ffff880100fc6880: fb fb fb fb fb fb fb fb fb fb fc fc fc fc fc fc
> [  321.270740] ==================================================================
> [  321.270903] Disabling lock debugging due to kernel taint
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101511
> Fixes: 7dd4f6729f92 ("drm/i915: Async GPU relocation processing")
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index eb46dfa374a7..a23a1c3503c8 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1199,7 +1199,7 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
>   	reservation_object_unlock(batch->resv);
>   	i915_vma_unpin(batch);
>   
> -	i915_vma_move_to_active(vma, rq, true);
> +	i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
>   	reservation_object_lock(vma->resv, NULL);
>   	reservation_object_add_excl_fence(vma->resv, &rq->fence);
>   	reservation_object_unlock(vma->resv);
> 

This one is straightforward.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko