• Chris Wilson's avatar
    drm/i915: Avoid accessing request->timeline outside of its lifetime · cb399eab
    Chris Wilson authored
    Whilst waiting on a request, we may do so without holding any locks or
    any guards beyond a reference to the request. In order to avoid taking
    locks within request deallocation, we drop references to its timeline
    (via the context and ppgtt) upon retirement. We should avoid chasing
    such pointers outside of their control, in particular we inspect the
    request->timeline to see if we may restore the RPS waitboost for a
    client. If we instead look at the engine->timeline, we will have similar
    behaviour on both full-ppgtt and !full-ppgtt systems and reduce the
    amount of reward we give towards stalling clients (i.e. only if the
    client stalls and the GPU is uncontended does it reclaim its boost).
    This restores behaviour back to pre-timelines, whilst fixing:
    
    [  645.078485] BUG: KASAN: use-after-free in i915_gem_object_wait_fence+0x1ee/0x2e0 at addr ffff8802335643a0
    [  645.078577] Read of size 4 by task gem_exec_schedu/28408
    [  645.078638] CPU: 1 PID: 28408 Comm: gem_exec_schedu Not tainted 4.9.0-rc2+ #64
    [  645.078724] Hardware name:                  /        , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
    [  645.078816]  ffff88022daef9a0 ffffffff8143d059 ffff880235402a80 ffff880233564200
    [  645.078998]  ffff88022daef9c8 ffffffff81229c5c ffff88022daefa48 ffff880233564200
    [  645.079172]  ffff880235402a80 ffff88022daefa38 ffffffff81229ef0 000000008110a796
    [  645.079345] Call Trace:
    [  645.079404]  [<ffffffff8143d059>] dump_stack+0x68/0x9f
    [  645.079467]  [<ffffffff81229c5c>] kasan_object_err+0x1c/0x70
    [  645.079534]  [<ffffffff81229ef0>] kasan_report_error+0x1f0/0x4b0
    [  645.079601]  [<ffffffff8122a244>] kasan_report+0x34/0x40
    [  645.079676]  [<ffffffff81634f5e>] ? i915_gem_object_wait_fence+0x1ee/0x2e0
    [  645.079741]  [<ffffffff81229951>] __asan_load4+0x61/0x80
    [  645.079807]  [<ffffffff81634f5e>] i915_gem_object_wait_fence+0x1ee/0x2e0
    [  645.079876]  [<ffffffff816364bf>] i915_gem_object_wait+0x19f/0x590
    [  645.079944]  [<ffffffff81636320>] ? i915_gem_object_wait_priority+0x500/0x500
    [  645.080016]  [<ffffffff8110fb30>] ? debug_show_all_locks+0x1e0/0x1e0
    [  645.080084]  [<ffffffff8110abdc>] ? check_chain_key+0x14c/0x210
    [  645.080157]  [<ffffffff8110a796>] ? __lock_is_held+0x46/0xc0
    [  645.080226]  [<ffffffff8163bc61>] ? i915_gem_set_domain_ioctl+0x141/0x690
    [  645.080296]  [<ffffffff8163bcc2>] i915_gem_set_domain_ioctl+0x1a2/0x690
    [  645.080366]  [<ffffffff811f8f85>] ? __might_fault+0x75/0xe0
    [  645.080433]  [<ffffffff815a55f7>] drm_ioctl+0x327/0x640
    [  645.080508]  [<ffffffff8163bb20>] ? i915_gem_obj_prepare_shmem_write+0x3a0/0x3a0
    [  645.080603]  [<ffffffff815a52d0>] ? drm_ioctl_permit+0x120/0x120
    [  645.080670]  [<ffffffff8110abdc>] ? check_chain_key+0x14c/0x210
    [  645.080738]  [<ffffffff81275717>] do_vfs_ioctl+0x127/0xa20
    [  645.080804]  [<ffffffff8120268c>] ? do_mmap+0x47c/0x580
    [  645.080871]  [<ffffffff811da567>] ? vm_mmap_pgoff+0x117/0x140
    [  645.080938]  [<ffffffff812755f0>] ? ioctl_preallocate+0x150/0x150
    [  645.081011]  [<ffffffff81108c53>] ? up_write+0x23/0x50
    [  645.081078]  [<ffffffff811da567>] ? vm_mmap_pgoff+0x117/0x140
    [  645.081145]  [<ffffffff811da450>] ? vma_is_stack_for_current+0x90/0x90
    [  645.081214]  [<ffffffff8110d853>] ? mark_held_locks+0x23/0xc0
    [  645.082030]  [<ffffffff81288408>] ? __fget+0x168/0x250
    [  645.082106]  [<ffffffff819ad517>] ? entry_SYSCALL_64_fastpath+0x5/0xb1
    [  645.082176]  [<ffffffff81288592>] ? __fget_light+0xa2/0xc0
    [  645.082242]  [<ffffffff8127604c>] SyS_ioctl+0x3c/0x70
    [  645.082309]  [<ffffffff819ad52e>] entry_SYSCALL_64_fastpath+0x1c/0xb1
    [  645.082374] Object at ffff880233564200, in cache kmalloc-8192 size: 8192
    [  645.082431] Allocated:
    [  645.082480] PID = 28408
    [  645.082535]  [  645.082566] [<ffffffff8103ae66>] save_stack_trace+0x16/0x20
    [  645.082623]  [  645.082656] [<ffffffff81228b06>] save_stack+0x46/0xd0
    [  645.082716]  [  645.082756] [<ffffffff812292fd>] kasan_kmalloc+0xad/0xe0
    [  645.082817]  [  645.082848] [<ffffffff81631752>] i915_ppgtt_create+0x52/0x220
    [  645.082908]  [  645.082941] [<ffffffff8161db96>] i915_gem_create_context+0x396/0x560
    [  645.083027]  [  645.083059] [<ffffffff8161f857>] i915_gem_context_create_ioctl+0x97/0xf0
    [  645.083152]  [  645.083183] [<ffffffff815a55f7>] drm_ioctl+0x327/0x640
    [  645.083243]  [  645.083274] [<ffffffff81275717>] do_vfs_ioctl+0x127/0xa20
    [  645.083334]  [  645.083372] [<ffffffff8127604c>] SyS_ioctl+0x3c/0x70
    [  645.083432]  [  645.083464] [<ffffffff819ad52e>] entry_SYSCALL_64_fastpath+0x1c/0xb1
    [  645.083551] Freed:
    [  645.083599] PID = 27629
    [  645.083648]  [  645.083676] [<ffffffff8103ae66>] save_stack_trace+0x16/0x20
    [  645.083738]  [  645.083770] [<ffffffff81228b06>] save_stack+0x46/0xd0
    [  645.083830]  [  645.083862] [<ffffffff81229203>] kasan_slab_free+0x73/0xc0
    [  645.083922]  [  645.083961] [<ffffffff812279c9>] kfree+0xa9/0x170
    [  645.084021]  [  645.084053] [<ffffffff81629f60>] i915_ppgtt_release+0x100/0x180
    [  645.084139]  [  645.084171] [<ffffffff8161d414>] i915_gem_context_free+0x1b4/0x230
    [  645.084257]  [  645.084288] [<ffffffff816537b2>] intel_lr_context_unpin+0x192/0x230
    [  645.084380]  [  645.084413] [<ffffffff81645250>] i915_gem_request_retire+0x620/0x630
    [  645.084500]  [  645.085226] [<ffffffff816473d1>] i915_gem_retire_requests+0x181/0x280
    [  645.085313]  [  645.085352] [<ffffffff816352ba>] i915_gem_retire_work_handler+0xca/0xe0
    [  645.085440]  [  645.085471] [<ffffffff810c725b>] process_one_work+0x4fb/0x920
    [  645.085532]  [  645.085562] [<ffffffff810c770d>] worker_thread+0x8d/0x840
    [  645.085622]  [  645.085653] [<ffffffff810d21e5>] kthread+0x185/0x1b0
    [  645.085718]  [  645.085750] [<ffffffff819ad7a7>] ret_from_fork+0x27/0x40
    [  645.085811] Memory state around the buggy address:
    [  645.085869]  ffff880233564280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    [  645.085956]  ffff880233564300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    [  645.086053] >ffff880233564380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    [  645.086138]                                ^
    [  645.086193]  ffff880233564400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    [  645.086283]  ffff880233564480: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    
    v2: Add a comment to document the hint like nature of
     intel_engine_last_submit()
    
    Fixes: 73cb9701 ("drm/i915: Combine seqno + tracking into a global timeline struct")
    Fixes: 80b204bc ("drm/i915: Enable multiple timelines")
    Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
    Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
    Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
    Link: http://patchwork.freedesktop.org/patch/msgid/20161101100317.11129-1-chris@chris-wilson.co.uk
    cb399eab
i915_gem.c 136 KB