linux-kernel - [PATCH] drm/atomic: swap_state should stall on cleanup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20191031223641.19208-1-robdclark@gmail.com>
Date:   Thu, 31 Oct 2019 15:36:41 -0700
From:   Rob Clark <robdclark@...il.com>
To:     dri-devel@...ts.freedesktop.org
Cc:     Rob Clark <robdclark@...omium.org>,
        Sean Paul <seanpaul@...omium.org>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Sean Paul <sean@...rly.run>, David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        linux-kernel@...r.kernel.org (open list)
Subject: [PATCH] drm/atomic: swap_state should stall on cleanup_done

From: Rob Clark <robdclark@...omium.org>

Stalling on cleanup_done ensures that any atomic state related to a
nonblock commit no longer has dangling references to per-object state
that can be freed.

Otherwise, if a !nonblock commit completes after a nonblock commit has
swapped state (ie. the synchronous part of the nonblock commit comes
before the !nonblock commit), but before the asynchronous part of the
nonblock commit completes, what was the new per-object state in the
nonblock commit can be freed.

This shows up with the new self-refresh helper, as _update_avg_times()
dereferences the original old and new crtc_state.

Fixes: d4da4e33341c ("drm: Measure Self Refresh Entry/Exit times to avoid thrashing")
Cc: Sean Paul <seanpaul@...omium.org>
Signed-off-by: Rob Clark <robdclark@...omium.org>
---
Other possibilities:
1) maybe block later before freeing atomic state?
2) refcount individual per-object state

 drivers/gpu/drm/drm_atomic_helper.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 3ef2ac52ce94..a5d95429f91b 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2711,7 +2711,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 			if (!commit)
 				continue;
 
-			ret = wait_for_completion_interruptible(&commit->hw_done);
+			ret = wait_for_completion_interruptible(&commit->cleanup_done);
 			if (ret)
 				return ret;
 		}
@@ -2722,7 +2722,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 			if (!commit)
 				continue;
 
-			ret = wait_for_completion_interruptible(&commit->hw_done);
+			ret = wait_for_completion_interruptible(&commit->cleanup_done);
 			if (ret)
 				return ret;
 		}
@@ -2733,7 +2733,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 			if (!commit)
 				continue;
 
-			ret = wait_for_completion_interruptible(&commit->hw_done);
+			ret = wait_for_completion_interruptible(&commit->cleanup_done);
 			if (ret)
 				return ret;
 		}
-- 
2.21.0