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@daenzer.net Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Andrey Grodzovsky andrey.grodzovsky@amd.com Cc: Harry Wentland harry.wentland@amd.com Signed-off-by: Nicholas Kazlauskas nicholas.kazlauskas@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;