Invoke drm_plane_helper_funcs.end_fb_access before drm_atomic_helper_commit_hw_done(). The latter function hands over ownership of the plane state to the following commit, which might free it. Releasing resources in end_fb_access then operates on undefined state. This bug has been observed with non-blocking commits when they are being queued up quickly. Here is an example stack trace from the bug report. The plane state has been free'd already, so the pages for drm_gem_fb_vunmap() are gone. Unable to handle kernel paging request at virtual address 0000000100000049 [...] drm_gem_fb_vunmap+0x18/0x74 drm_gem_end_shadow_fb_access+0x1c/0x2c drm_atomic_helper_cleanup_planes+0x58/0xd8 drm_atomic_helper_commit_tail+0x90/0xa0 commit_tail+0x15c/0x188 commit_work+0x14/0x20 For aborted commits, it is still ok to run end_fb_access as part of the plane's cleanup. Add a test to drm_atomic_helper_cleanup_planes(). v2: * fix test in drm_atomic_helper_cleanup_planes() Reported-by: Alyssa Ross <hi@xxxxxxxxx> Closes: https://lore.kernel.org/dri-devel/87leazm0ya.fsf@xxxxxxxxx/ Suggested-by: Daniel Vetter <daniel@xxxxxxxx> Fixes: 94d879eaf7fb ("drm/atomic-helper: Add {begin,end}_fb_access to plane helpers") Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> Cc: <stable@xxxxxxxxxxxxxxx> # v6.2+ --- drivers/gpu/drm/drm_atomic_helper.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index c3f677130def0..bedb42ddd1341 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2784,6 +2784,17 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, funcs->atomic_flush(crtc, old_state); } + + /* + * Signal end of framebuffer access here before hw_done. After hw_done, + * a later commit might have already released the plane state. + */ + for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, new_plane_state, i) { + const struct drm_plane_helper_funcs *funcs = plane->helper_private; + + if (funcs->end_fb_access) + funcs->end_fb_access(plane, new_plane_state); + } } EXPORT_SYMBOL(drm_atomic_helper_commit_planes); @@ -2924,6 +2935,12 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev, for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, new_plane_state, i) { const struct drm_plane_helper_funcs *funcs = plane->helper_private; + /* + * Only clean up here if we're aborting the commit. + */ + if (old_plane_state == plane->state) + continue; + if (funcs->end_fb_access) funcs->end_fb_access(plane, new_plane_state); } -- 2.43.0
