| From 2216322919c8608a448d7ebc560a845238a5d6b6 Mon Sep 17 00:00:00 2001 |
| From: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> |
| Date: Mon, 7 Jan 2019 12:41:46 -0500 |
| Subject: drm: Block fb changes for async plane updates |
| |
| From: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> |
| |
| commit 2216322919c8608a448d7ebc560a845238a5d6b6 upstream. |
| |
| 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: fef9df8b5945 ("drm/atomic: initial support for asynchronous plane update") |
| Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> |
| Acked-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com> |
| Acked-by: Harry Wentland <harry.wentland@amd.com> |
| Reviewed-by: Daniel Vetter <daniel@ffwll.ch> |
| Signed-off-by: Harry Wentland <harry.wentland@amd.com> |
| Link: https://patchwork.freedesktop.org/patch/275364/ |
| Signed-off-by: Dave Airlie <airlied@redhat.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/gpu/drm/drm_atomic_helper.c | 9 +++++++++ |
| 1 file changed, 9 insertions(+) |
| |
| --- a/drivers/gpu/drm/drm_atomic_helper.c |
| +++ b/drivers/gpu/drm/drm_atomic_helper.c |
| @@ -1564,6 +1564,15 @@ int drm_atomic_helper_async_check(struct |
| old_plane_state->crtc != new_plane_state->crtc) |
| return -EINVAL; |
| |
| + /* |
| + * FIXME: Since prepare_fb and cleanup_fb are always called on |
| + * the new_plane_state for async updates we need to block framebuffer |
| + * changes. This prevents use of a fb that's been cleaned up and |
| + * double cleanups from occuring. |
| + */ |
| + if (old_plane_state->fb != new_plane_state->fb) |
| + return -EINVAL; |
| + |
| funcs = plane->helper_private; |
| if (!funcs->atomic_async_update) |
| return -EINVAL; |