• Nicholas Kazlauskas's avatar
    drm/amd/display: Skip fast cursor updates for fb changes · 77acd1cd
    Nicholas Kazlauskas authored
    [Why]
    The behavior of drm_atomic_helper_cleanup_planes differs depending on
    whether the commit was asynchronous or not. When it's called from
    amdgpu_dm_atomic_commit_tail during a typical atomic commit the
    plane state has been swapped so it calls cleanup_fb on the old plane
    state.
    
    However, in the asynchronous commit codepath the call to
    drm_atomic_helper_commit also calls dm_plane_helper_cleanup_fb after
    atomic_async_update has been called. Since the plane state is updated
    in place and has not been swapped the cleanup_fb call affects the new
    plane state.
    
    This results in a use after free for the given sequence:
    
    - Fast update, fb1 pin/ref, fb1 unpin/unref
    - Fast update, fb2 pin/ref, fb2 unpin/unref
    - Slow update, fb1 pin/ref, fb2 unpin/unref
    - Fast update, fb2 pin/ref -> use after free. bug
    
    [How]
    Disallow framebuffer changes in the fast path. Since this includes
    a NULL framebuffer, this means that only framebuffers that have
    been previously pin+ref at least once will be used, preventing a
    use after free.
    
    This has a significant throughput reduction for cursor updates where
    the framebuffer changes. For most desktop usage this isn't a problem,
    but it does introduce performance regressions for two specific IGT
    tests:
    
    - cursor-vs-flip-toggle
    - cursor-vs-flip-varying-size
    
    Fixes: 2cc751931afc ("drm/amd/display: Add fast path for cursor plane updates")
    Signed-off-by: default avatarNicholas Kazlauskas <nicholas.kazlauskas@amd.com>
    Reviewed-by: default avatarAndrey Grodzovsky <andrey.grodzovsky@amd.com>
    Reviewed-by: default avatarHarry Wentland <harry.wentland@amd.com>
    Signed-off-by: default avatarAlex Deucher <alexander.deucher@amd.com>
    77acd1cd
amdgpu_dm.c 167 KB