[PATCH] drm: Block fb changes for async plane updates

[Nicholas Kazlauskas]

The behavior of drm_atomic_helper_cleanup_planes differs depending on
whether the commit was an asynchronous update or not.

For a typical (non-async) atomic commit prepare_fb is called on the
new_plane_state and cleanup_fb is called on the old_plane_state.

However, async commits are performed in place and don't swap the state
for the plane. The call to prepare_fb happens on the new_plane_state
and the call to cleanup_fb is also called on the new_plane_state in
this case (since the state hasn't swapped).

This behavior can lead to use-after-free or unpin of an active fb.

Consider the following sequence of events for interleaving fbs:

- Async update, fb1 prepare, fb1 cleanup_fb
- Async update, fb2 prepare, fb2 cleanup_fb
- Non-async update, fb1 prepare, fb2 cleanup_fb
- Async update, fb2 cleanup_fb -> double cleanup, use-after-free

This situation has been observed in practice for a double buffered
cursor when closing an X client. The non-async update occurs because
the new_plane_state->crtc != old_plane_state->crtc which forces the
non-async path to occur.

The simplest fix for this is to block fb updates in
drm_atomic_helper_async_check. This guarantees that the framebuffer
will have previously been prepared and any subsequent async updates
will always call prepare and cleanup_fb like the non-async atomic
commit path would.

Cc: Michel Dänzer <michel at daenzer.net>
Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
Cc: Andrey Grodzovsky <andrey.grodzovsky at amd.com>
Cc: Harry Wentland <harry.wentland at amd.com>
Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 54e2ae614dcc..d2f80bf14f86 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1599,7 +1599,8 @@ int drm_atomic_helper_async_check(struct drm_device *dev,
 		return -EINVAL;
 
 	if (!new_plane_state->crtc ||
-	    old_plane_state->crtc != new_plane_state->crtc)
+	    old_plane_state->crtc != new_plane_state->crtc ||
+	    old_plane_state->fb != new_plane_state->fb)
 		return -EINVAL;
 
 	funcs = plane->helper_private;
-- 
2.17.1