• Nicholas Kazlauskas's avatar
    drm: Block fb changes for async plane updates · 25dc194b
    Nicholas Kazlauskas authored
    The prepare_fb call always happens on new_plane_state.
    
    The drm_atomic_helper_cleanup_planes checks to see if
    plane state pointer has changed when deciding to call cleanup_fb on
    either the new_plane_state or the old_plane_state.
    
    For a non-async atomic commit the state pointer is swapped, so this
    helper calls prepare_fb on the new_plane_state and cleanup_fb on the
    old_plane_state. This makes sense, since we want to prepare the
    framebuffer we are going to use and cleanup the the framebuffer we are
    no longer using.
    
    For the async atomic update helpers this differs. The async atomic
    update helpers perform in-place updates on the existing state. They call
    drm_atomic_helper_cleanup_planes but the state pointer is not swapped.
    This means that prepare_fb is called on the new_plane_state and
    cleanup_fb is called on the new_plane_state (not the old).
    
    In the case where old_plane_state->fb == new_plane_state->fb then
    there should be no behavioral difference between an async update
    and a non-async commit. But there are issues that arise when
    old_plane_state->fb != new_plane_state->fb.
    
    The first is that the new_plane_state->fb is immediately cleaned up
    after it has been prepared, so we're using a fb that we shouldn't
    be.
    
    The second occurs during a sequence of async atomic updates and
    non-async regular atomic commits. Suppose there are two framebuffers
    being interleaved in a double-buffering scenario, fb1 and fb2:
    
    - Async update, oldfb = NULL, newfb = fb1, prepare fb1, cleanup fb1
    - Async update, oldfb = fb1, newfb = fb2, prepare fb2, cleanup fb2
    - Non-async commit, oldfb = fb2, newfb = fb1, prepare fb1, cleanup fb2
    
    We call cleanup_fb on fb2 twice in this example scenario, and any
    further use will result in use-after-free.
    
    The simple fix to this problem is to block framebuffer changes
    in the drm_atomic_helper_async_check function for now.
    
    v2: Move check by itself, add a FIXME (Daniel)
    
    Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
    Cc: Harry Wentland <harry.wentland@amd.com>
    Cc: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
    Cc: <stable@vger.kernel.org> # v4.14+
    Fixes: fef9df8b ("drm/atomic: initial support for asynchronous plane update")
    Signed-off-by: default avatarNicholas Kazlauskas <nicholas.kazlauskas@amd.com>
    Acked-by: default avatarAndrey Grodzovsky <andrey.grodzovsky@amd.com>
    Acked-by: default avatarHarry Wentland <harry.wentland@amd.com>
    Reviewed-by: default avatarDaniel Vetter <daniel@ffwll.ch>
    Signed-off-by: default avatarHarry Wentland <harry.wentland@amd.com>
    Link: https://patchwork.freedesktop.org/patch/275364/
    25dc194b
drm_atomic_helper.c 108 KB