linux-kernel - [PATCH AUTOSEL for 4.9 41/56] drm/i915: Protect dev_priv->atomic_cdclk

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171115024521.5884-41-alexander.levin@verizon.com>
Date:   Wed, 15 Nov 2017 02:45:44 +0000
From:   alexander.levin@...izon.com
To:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Cc:     Ville Syrjälä <ville.syrjala@...ux.intel.com>,
        "Maarten Lankhorst" <maarten.lankhorst@...ux.intel.com>,
        alexander.levin@...izon.com
Subject: [PATCH AUTOSEL for 4.9 41/56] drm/i915: Protect
 dev_priv->atomic_cdclk_freq with all the crtc locks

From: Ville Syrjälä <ville.syrjala@...ux.intel.com>

[ Upstream commit 8d96561a0a31974fec33bd3f3853d6211b7ab738 ]

A modeset on one pipe can update dev_priv->atomic_cdclk_freq without
actually touching the hardware, in which case we won't force a modeset
on all the pipes, and thus won't lock any of the other pipes either.
That means a parallel plane update on another pipe could be looking at
a stale dev_priv->atomic_cdcdlk_freq and thus fail to notice when the
plane configuration is invalid, or potentially reject a valid update.

To overcome this we must protect writes to atomic_cdclk_freq with
all the crtc locks, and thus for reads any single crtc lock will
be sufficient protection.

Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@...ux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1479141311-11904-3-git-send-email-ville.syrjala@linux.intel.com
Reviewed-by: Daniel Vetter <daniel.vetter@...ll.ch>
Signed-off-by: Sasha Levin <alexander.levin@...izon.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  9 +++++++-
 drivers/gpu/drm/i915/intel_display.c | 41 +++++++++++++++++++++++++++++++-----
 2 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d120797aec17..b9e759ba16c2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1841,7 +1841,14 @@ struct drm_i915_private {
 
 	unsigned int fsb_freq, mem_freq, is_ddr3;
 	unsigned int skl_preferred_vco_freq;
-	unsigned int cdclk_freq, max_cdclk_freq, atomic_cdclk_freq;
+	unsigned int cdclk_freq, max_cdclk_freq;
+
+	/*
+	 * For reading holding any crtc lock is sufficient,
+	 * for writing must hold all of them.
+	 */
+	unsigned int atomic_cdclk_freq;
+
 	unsigned int max_dotclk_freq;
 	unsigned int rawclk_freq;
 	unsigned int hpll_freq;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ce32303b3013..bd560a8792ba 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13894,13 +13894,32 @@ static int haswell_mode_set_planes_workaround(struct drm_atomic_state *state)
 	return 0;
 }
 
+static int intel_lock_all_pipes(struct drm_atomic_state *state)
+{
+	struct drm_crtc *crtc;
+
+	/* Add all pipes to the state */
+	for_each_crtc(state->dev, crtc) {
+		struct drm_crtc_state *crtc_state;
+
+		crtc_state = drm_atomic_get_crtc_state(state, crtc);
+		if (IS_ERR(crtc_state))
+			return PTR_ERR(crtc_state);
+	}
+
+	return 0;
+}
+
 static int intel_modeset_all_pipes(struct drm_atomic_state *state)
 {
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *crtc_state;
 	int ret = 0;
 
-	/* add all active pipes to the state */
+	/*
+	 * Add all pipes to the state, and force
+	 * a modeset on all the active ones.
+	 */
 	for_each_crtc(state->dev, crtc) {
 		crtc_state = drm_atomic_get_crtc_state(state, crtc);
 		if (IS_ERR(crtc_state))
@@ -13966,12 +13985,24 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
 		if (ret < 0)
 			return ret;
 
+		/*
+		 * Writes to dev_priv->atomic_cdclk_freq must protected by
+		 * holding all the crtc locks, even if we don't end up
+		 * touching the hardware
+		 */
+		if (intel_state->cdclk != dev_priv->atomic_cdclk_freq) {
+			ret = intel_lock_all_pipes(state);
+			if (ret < 0)
+				return ret;
+		}
+
+		/* All pipes must be switched off while we change the cdclk. */
 		if (intel_state->dev_cdclk != dev_priv->cdclk_freq ||
-		    intel_state->cdclk_pll_vco != dev_priv->cdclk_pll.vco)
+		    intel_state->cdclk_pll_vco != dev_priv->cdclk_pll.vco) {
 			ret = intel_modeset_all_pipes(state);
-
-		if (ret < 0)
-			return ret;
+			if (ret < 0)
+				return ret;
+		}
 
 		DRM_DEBUG_KMS("New cdclk calculated to be atomic %u, actual %u\n",
 			      intel_state->cdclk, intel_state->dev_cdclk);
-- 
2.11.0