All of lore.kernel.org
 help / color / mirror / Atom feed
From: Imre Deak <imre.deak@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: "Mat Jonczyk" <mat.jonczyk@o2.pl>,
	"José Roberto de Souza" <jose.souza@intel.com>,
	"Jani Nikula" <jani.nikula@intel.com>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	stable@vger.kernel.org
Subject: [Intel-gfx] [PATCH 1/6] drm/i915/dp: Skip the HW readout of DPCD on disabled encoders
Date: Mon, 18 Oct 2021 12:41:49 +0300	[thread overview]
Message-ID: <20211018094154.1407705-2-imre.deak@intel.com> (raw)
In-Reply-To: <20211018094154.1407705-1-imre.deak@intel.com>

Reading out the DP encoders' DPCD during booting or resume is only
required for enabled encoders: such encoders may be modesetted during
the initial commit and the link training this involves depends on an
initialized DPCD. For DDI encoders reading out the DPCD is skipped, do
the same on pre-DDI platforms.

Atm, the first DPCD readout without a sink connected - which is a likely
scneario if the encoder is disabled - leaves intel_dp->num_common_rates
at 0, which resulted in

intel_dp_sync_state()->intel_dp_max_common_rate()

in a

intel_dp->common_rates[-1]

access. This by definition results in an undefined behaviour, though to
my best knowledge in all HW/compiler configurations it actually results
in accessing the array item type value preceding the array. In this
case the preceding value happens to be intel_dp->num_common_rates,
which is 0, so this issue - by luck - didn't cause a user visible
problem.

Nevertheless it's still an undefined behaviour and in CONFIG_UBSAN
builds leads to a kernel BUG() (which revealed this problem for us),
hence CC:stable.

A related problem in case the encoder is enabled but the sink is not
connected or the DPCD readout fails is fixed by the next patch.

v2: Amend the commit message describing the root cause of the
    CONFIG_UBSAN BUG().

Fixes: a532cde31de3 ("drm/i915/tc: Fix TypeC port init/resume time sanitization")
References: https://gitlab.freedesktop.org/drm/intel/-/issues/4297
Reported-and-tested-by: Mat Jonczyk <mat.jonczyk@o2.pl>
Cc: Mat Jonczyk <mat.jonczyk@o2.pl>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 9d8132dd4cc5a..23de500d56b52 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2007,6 +2007,9 @@ void intel_dp_sync_state(struct intel_encoder *encoder,
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
+	if (!crtc_state)
+		return;
+
 	/*
 	 * Don't clobber DPCD if it's been already read out during output
 	 * setup (eDP) or detect.
-- 
2.27.0


WARNING: multiple messages have this Message-ID (diff)
From: Imre Deak <imre.deak@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: "Mat Jonczyk" <mat.jonczyk@o2.pl>,
	"José Roberto de Souza" <jose.souza@intel.com>,
	"Jani Nikula" <jani.nikula@intel.com>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	stable@vger.kernel.org
Subject: [PATCH 1/6] drm/i915/dp: Skip the HW readout of DPCD on disabled encoders
Date: Mon, 18 Oct 2021 12:41:49 +0300	[thread overview]
Message-ID: <20211018094154.1407705-2-imre.deak@intel.com> (raw)
In-Reply-To: <20211018094154.1407705-1-imre.deak@intel.com>

Reading out the DP encoders' DPCD during booting or resume is only
required for enabled encoders: such encoders may be modesetted during
the initial commit and the link training this involves depends on an
initialized DPCD. For DDI encoders reading out the DPCD is skipped, do
the same on pre-DDI platforms.

Atm, the first DPCD readout without a sink connected - which is a likely
scneario if the encoder is disabled - leaves intel_dp->num_common_rates
at 0, which resulted in

intel_dp_sync_state()->intel_dp_max_common_rate()

in a

intel_dp->common_rates[-1]

access. This by definition results in an undefined behaviour, though to
my best knowledge in all HW/compiler configurations it actually results
in accessing the array item type value preceding the array. In this
case the preceding value happens to be intel_dp->num_common_rates,
which is 0, so this issue - by luck - didn't cause a user visible
problem.

Nevertheless it's still an undefined behaviour and in CONFIG_UBSAN
builds leads to a kernel BUG() (which revealed this problem for us),
hence CC:stable.

A related problem in case the encoder is enabled but the sink is not
connected or the DPCD readout fails is fixed by the next patch.

v2: Amend the commit message describing the root cause of the
    CONFIG_UBSAN BUG().

Fixes: a532cde31de3 ("drm/i915/tc: Fix TypeC port init/resume time sanitization")
References: https://gitlab.freedesktop.org/drm/intel/-/issues/4297
Reported-and-tested-by: Mat Jonczyk <mat.jonczyk@o2.pl>
Cc: Mat Jonczyk <mat.jonczyk@o2.pl>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 9d8132dd4cc5a..23de500d56b52 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2007,6 +2007,9 @@ void intel_dp_sync_state(struct intel_encoder *encoder,
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
+	if (!crtc_state)
+		return;
+
 	/*
 	 * Don't clobber DPCD if it's been already read out during output
 	 * setup (eDP) or detect.
-- 
2.27.0


  reply	other threads:[~2021-10-18  9:42 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-18  9:41 [Intel-gfx] [PATCH 0/6] drm/i915/dp: Fix link parameter use in lack of a valid DPCD Imre Deak
2021-10-18  9:41 ` Imre Deak [this message]
2021-10-18  9:41   ` [PATCH 1/6] drm/i915/dp: Skip the HW readout of DPCD on disabled encoders Imre Deak
2021-10-18  9:41 ` [Intel-gfx] [PATCH 2/6] drm/i915/dp: Ensure sink rate values are always valid Imre Deak
2021-10-18  9:41   ` Imre Deak
2021-10-18 14:34   ` [PATCH v2 " Imre Deak
2021-10-18 14:34     ` [Intel-gfx] " Imre Deak
2021-10-19  7:27   ` [Intel-gfx] [PATCH " Jani Nikula
2021-10-19  7:33     ` Imre Deak
2021-10-19  7:37       ` Jani Nikula
2021-10-19  7:39         ` Imre Deak
2021-10-19 18:37           ` Imre Deak
2021-10-19 19:17             ` Jani Nikula
2021-10-18  9:41 ` [Intel-gfx] [PATCH 3/6] drm/i915/dp: Ensure max link params " Imre Deak
2021-10-18  9:41   ` Imre Deak
2021-10-18  9:41 ` [Intel-gfx] [PATCH 4/6] drm/i915/dp: Ensure sink/link max lane count values " Imre Deak
2021-10-18 15:04   ` Ville Syrjälä
2021-10-18 15:13     ` Imre Deak
2021-10-18 15:27       ` Ville Syrjälä
2021-10-18  9:41 ` [Intel-gfx] [PATCH 5/6] drm/i915/dp: Sanitize sink rate DPCD register values Imre Deak
2021-10-18  9:41 ` [Intel-gfx] [PATCH 6/6] drm/i915/dp: Sanitize link common rate array lookups Imre Deak
2021-10-19 19:23   ` Jani Nikula
2021-10-20  9:06     ` Imre Deak
2021-10-20  9:53       ` Jani Nikula
2021-10-20 10:09         ` Ville Syrjälä
2021-10-18 12:31 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dp: Fix link parameter use in lack of a valid DPCD Patchwork
2021-10-18 12:33 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-10-18 13:06 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2021-10-18 18:01 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dp: Fix link parameter use in lack of a valid DPCD (rev2) Patchwork
2021-10-18 18:03 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-10-18 18:31 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-10-19  0:52 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2021-10-19 12:54   ` Imre Deak
2021-10-19 15:33     ` Vudum, Lakshminarayana
2021-10-19 16:32       ` Imre Deak
2021-10-19 14:45 ` [Intel-gfx] ✓ Fi.CI.IGT: success " Patchwork
2021-10-20 15:40   ` Imre Deak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211018094154.1407705-2-imre.deak@intel.com \
    --to=imre.deak@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=jose.souza@intel.com \
    --cc=mat.jonczyk@o2.pl \
    --cc=stable@vger.kernel.org \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.