* [PATCH 1/5] drm/i915/opregion: check port number bounds for SWSCI display power state [not found] <cover.1642072583.git.jani.nikula@intel.com> @ 2022-01-13 11:18 ` Jani Nikula 2022-01-13 16:42 ` [Intel-gfx] " Lucas De Marchi 0 siblings, 1 reply; 3+ messages in thread From: Jani Nikula @ 2022-01-13 11:18 UTC (permalink / raw) To: intel-gfx; +Cc: jani.nikula, ville.syrjala, stable The mapping from enum port to whatever port numbering scheme is used by the SWSCI Display Power State Notification is odd, and the memory of it has faded. In any case, the parameter only has space for ports numbered [0..4], and UBSAN reports bit shift beyond it when the platform has port F or more. Since the SWSCI functionality is supposed to be obsolete for new platforms (i.e. ones that might have port F or more), just bail out early if the mapped and mangled port number is beyond what the Display Power State Notification can support. Fixes: 9c4b0a683193 ("drm/i915: add opregion function to notify bios of encoder enable/disable") Cc: <stable@vger.kernel.org> # v3.13+ Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4800 Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/display/intel_opregion.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c index af9d30f56cc1..ad1afe9df6c3 100644 --- a/drivers/gpu/drm/i915/display/intel_opregion.c +++ b/drivers/gpu/drm/i915/display/intel_opregion.c @@ -363,6 +363,21 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder, port++; } + /* + * The port numbering and mapping here is bizarre. The now-obsolete + * swsci spec supports ports numbered [0..4]. Port E is handled as a + * special case, but port F and beyond are not. The functionality is + * supposed to be obsolete for new platforms. Just bail out if the port + * number is out of bounds after mapping. + */ + if (port > 4) { + drm_dbg_kms(&dev_priv->drm, + "[ENCODER:%d:%s] port %c (index %u) out of bounds for display power state notification\n", + intel_encoder->base.base.id, intel_encoder->base.name, + port_name(intel_encoder->port), port); + return -EINVAL; + } + if (!enable) parm |= 4 << 8; -- 2.30.2 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Intel-gfx] [PATCH 1/5] drm/i915/opregion: check port number bounds for SWSCI display power state 2022-01-13 11:18 ` [PATCH 1/5] drm/i915/opregion: check port number bounds for SWSCI display power state Jani Nikula @ 2022-01-13 16:42 ` Lucas De Marchi 2022-01-13 16:53 ` Jani Nikula 0 siblings, 1 reply; 3+ messages in thread From: Lucas De Marchi @ 2022-01-13 16:42 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx, stable On Thu, Jan 13, 2022 at 01:18:03PM +0200, Jani Nikula wrote: >The mapping from enum port to whatever port numbering scheme is used by >the SWSCI Display Power State Notification is odd, and the memory of it >has faded. In any case, the parameter only has space for ports numbered >[0..4], and UBSAN reports bit shift beyond it when the platform has port >F or more. > >Since the SWSCI functionality is supposed to be obsolete for new >platforms (i.e. ones that might have port F or more), just bail out >early if the mapped and mangled port number is beyond what the Display >Power State Notification can support. > >Fixes: 9c4b0a683193 ("drm/i915: add opregion function to notify bios of encoder enable/disable") >Cc: <stable@vger.kernel.org> # v3.13+ >Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4800 >Signed-off-by: Jani Nikula <jani.nikula@intel.com> >--- > drivers/gpu/drm/i915/display/intel_opregion.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > >diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c >index af9d30f56cc1..ad1afe9df6c3 100644 >--- a/drivers/gpu/drm/i915/display/intel_opregion.c >+++ b/drivers/gpu/drm/i915/display/intel_opregion.c >@@ -363,6 +363,21 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder, > port++; > } > >+ /* >+ * The port numbering and mapping here is bizarre. The now-obsolete >+ * swsci spec supports ports numbered [0..4]. Port E is handled as a >+ * special case, but port F and beyond are not. The functionality is >+ * supposed to be obsolete for new platforms. Just bail out if the port >+ * number is out of bounds after mapping. >+ */ >+ if (port > 4) { >+ drm_dbg_kms(&dev_priv->drm, >+ "[ENCODER:%d:%s] port %c (index %u) out of bounds for display power state notification\n", >+ intel_encoder->base.base.id, intel_encoder->base.name, >+ port_name(intel_encoder->port), port); Do we need this log message? It will always trigger for platforms with PORT F and callers simply ignore the return value. Lucas De Marchi ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Intel-gfx] [PATCH 1/5] drm/i915/opregion: check port number bounds for SWSCI display power state 2022-01-13 16:42 ` [Intel-gfx] " Lucas De Marchi @ 2022-01-13 16:53 ` Jani Nikula 0 siblings, 0 replies; 3+ messages in thread From: Jani Nikula @ 2022-01-13 16:53 UTC (permalink / raw) To: Lucas De Marchi; +Cc: intel-gfx, stable On Thu, 13 Jan 2022, Lucas De Marchi <lucas.demarchi@intel.com> wrote: > On Thu, Jan 13, 2022 at 01:18:03PM +0200, Jani Nikula wrote: >>The mapping from enum port to whatever port numbering scheme is used by >>the SWSCI Display Power State Notification is odd, and the memory of it >>has faded. In any case, the parameter only has space for ports numbered >>[0..4], and UBSAN reports bit shift beyond it when the platform has port >>F or more. >> >>Since the SWSCI functionality is supposed to be obsolete for new >>platforms (i.e. ones that might have port F or more), just bail out >>early if the mapped and mangled port number is beyond what the Display >>Power State Notification can support. >> >>Fixes: 9c4b0a683193 ("drm/i915: add opregion function to notify bios of encoder enable/disable") >>Cc: <stable@vger.kernel.org> # v3.13+ >>Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >>Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4800 >>Signed-off-by: Jani Nikula <jani.nikula@intel.com> >>--- >> drivers/gpu/drm/i915/display/intel_opregion.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >>diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c >>index af9d30f56cc1..ad1afe9df6c3 100644 >>--- a/drivers/gpu/drm/i915/display/intel_opregion.c >>+++ b/drivers/gpu/drm/i915/display/intel_opregion.c >>@@ -363,6 +363,21 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder, >> port++; >> } >> >>+ /* >>+ * The port numbering and mapping here is bizarre. The now-obsolete >>+ * swsci spec supports ports numbered [0..4]. Port E is handled as a >>+ * special case, but port F and beyond are not. The functionality is >>+ * supposed to be obsolete for new platforms. Just bail out if the port >>+ * number is out of bounds after mapping. >>+ */ >>+ if (port > 4) { >>+ drm_dbg_kms(&dev_priv->drm, >>+ "[ENCODER:%d:%s] port %c (index %u) out of bounds for display power state notification\n", >>+ intel_encoder->base.base.id, intel_encoder->base.name, >>+ port_name(intel_encoder->port), port); > > Do we need this log message? It will always trigger for platforms with > PORT F and callers simply ignore the return value. It will trigger for patch 1, and the stable backport, but, with any luck and proper opregions, it will be silenced by the subsequent patches. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-01-13 16:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1642072583.git.jani.nikula@intel.com>
2022-01-13 11:18 ` [PATCH 1/5] drm/i915/opregion: check port number bounds for SWSCI display power state Jani Nikula
2022-01-13 16:42 ` [Intel-gfx] " Lucas De Marchi
2022-01-13 16:53 ` Jani Nikula
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).