Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits

During a non-blocking commit, it is possible to return before the
commit_tail work is queued (-ERESTARTSYS, for example).

Since a reference on the crtc commit object is obtained for the pending
vblank event when preparing the commit, the above situation will leave
us with an extra reference.

Therefore, if the commit_tail worker has not consumed the event at the
end of a commit, release it's reference.

Changes since v1:
- Also check for state->event->base.completion being set, to
handle the case where stall_checks() fails in setup_crtc_commit().
Changes since v2:
- Add a flag to drm_crtc_commit, to prevent dereferencing a freed event.
i915 may unreference the state in a worker.

Fixes: 24835e442f28 ("drm: reference count event->completion")
Cc: <stable@vger.kernel.org> # v4.11+
Signed-off-by: Leo (Sunpeng) Li <sunpeng.li@amd.com>
Acked-by: Harry Wentland <harry.wentland@amd.com> #v1
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180117115108.29608-1-maarten.lankhorst@linux.intel.com
Reviewed-by: Sean Paul <seanpaul@chromium.org>

authored by

Leo (Sunpeng) Li and committed by
Maarten Lankhorst
54f809cf 745fd50f

+24
+15
drivers/gpu/drm/drm_atomic_helper.c
··· 1778 1778 new_crtc_state->event->base.completion = &commit->flip_done; 1779 1779 new_crtc_state->event->base.completion_release = release_crtc_commit; 1780 1780 drm_crtc_commit_get(commit); 1781 + 1782 + commit->abort_completion = true; 1781 1783 } 1782 1784 1783 1785 for_each_oldnew_connector_in_state(state, conn, old_conn_state, new_conn_state, i) { ··· 3329 3327 void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state) 3330 3328 { 3331 3329 if (state->commit) { 3330 + /* 3331 + * In the event that a non-blocking commit returns 3332 + * -ERESTARTSYS before the commit_tail work is queued, we will 3333 + * have an extra reference to the commit object. Release it, if 3334 + * the event has not been consumed by the worker. 3335 + * 3336 + * state->event may be freed, so we can't directly look at 3337 + * state->event->base.completion. 3338 + */ 3339 + if (state->event && state->commit->abort_completion) 3340 + drm_crtc_commit_put(state->commit); 3341 + 3332 3342 kfree(state->commit->event); 3333 3343 state->commit->event = NULL; 3344 + 3334 3345 drm_crtc_commit_put(state->commit); 3335 3346 } 3336 3347
+9
include/drm/drm_atomic.h
··· 134 134 * &drm_pending_vblank_event pointer to clean up private events. 135 135 */ 136 136 struct drm_pending_vblank_event *event; 137 + 138 + /** 139 + * @abort_completion: 140 + * 141 + * A flag that's set after drm_atomic_helper_setup_commit takes a second 142 + * reference for the completion of $drm_crtc_state.event. It's used by 143 + * the free code to remove the second reference if commit fails. 144 + */ 145 + bool abort_completion; 137 146 }; 138 147 139 148 struct __drm_planes_state {