intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH] drm: i915: do not NULL deref hdmi attached_connector
@ 2024-10-31 10:51 Sergey Senozhatsky
  2024-10-31 11:20 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Sergey Senozhatsky @ 2024-10-31 10:51 UTC (permalink / raw)
  To: David Airlie, Simona Vetter
  Cc: Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
	intel-gfx, intel-xe, dri-devel, linux-kernel, Sergey Senozhatsky

	*** RFC ***

intel_ddi_init() may skip connector initialization, for instance,
both intel_ddi_init_dp_connector() and intel_ddi_init_hdmi_connector()
are optional.  This leads to situation that ->attached_connector may
be NULL for some connectors.  For instance, on my setup 'DDI A/PHY A'
and 'DDI TC1/PHY TC1' are not initialized.

However, functions like intel_dp_dual_mode_set_tmds_output() and
friends don't take this into consideration.  This leads to NULL
ptr-derefs:

KASAN: null-ptr-deref in range [0x0000000000000848-0x000000000000084f]
RIP: 0010:intel_hdmi_encoder_shutdown+0x105/0x230
Call Trace:
<TASK>
i915_driver_shutdown+0x2d8/0x490
pci_device_shutdown+0x83/0x150
device_shutdown+0x4ad/0x660
__se_sys_reboot+0x29c/0x4d0
do_syscall_64+0x60/0x90

Add a new helper to avoid NULL ->attached_connector derefs and
switch some intel_hdmi function to it.  I'm not sure if we need
to switch all or just intel_dp_dual_mode_set_tmds_output() (I
have only seen this one doing NULL derefs so far).

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 27 ++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index e1a1351bc94f..c089dd20972b 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1256,12 +1256,19 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
 			      &crtc_state->infoframes.drm);
 }
 
+static struct i2c_adapter *to_ddc(struct intel_hdmi *hdmi)
+{
+	if (hdmi->attached_connector)
+		return hdmi->attached_connector->base.ddc;
+	return NULL;
+}
+
 void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable)
 {
 	struct intel_display *display = to_intel_display(hdmi);
-	struct i2c_adapter *ddc = hdmi->attached_connector->base.ddc;
+	struct i2c_adapter *ddc = to_ddc(hdmi);
 
-	if (hdmi->dp_dual_mode.type < DRM_DP_DUAL_MODE_TYPE2_DVI)
+	if (!ddc || hdmi->dp_dual_mode.type < DRM_DP_DUAL_MODE_TYPE2_DVI)
 		return;
 
 	drm_dbg_kms(display->drm, "%s DP dual mode adaptor TMDS output\n",
@@ -1275,7 +1282,7 @@ static int intel_hdmi_hdcp_read(struct intel_digital_port *dig_port,
 				unsigned int offset, void *buffer, size_t size)
 {
 	struct intel_hdmi *hdmi = &dig_port->hdmi;
-	struct i2c_adapter *ddc = hdmi->attached_connector->base.ddc;
+	struct i2c_adapter *ddc = to_ddc(hdmi);
 	int ret;
 	u8 start = offset & 0xff;
 	struct i2c_msg msgs[] = {
@@ -1292,6 +1299,10 @@ static int intel_hdmi_hdcp_read(struct intel_digital_port *dig_port,
 			.buf = buffer
 		}
 	};
+
+	if (!ddc)
+		return -EINVAL;
+
 	ret = i2c_transfer(ddc, msgs, ARRAY_SIZE(msgs));
 	if (ret == ARRAY_SIZE(msgs))
 		return 0;
@@ -1302,11 +1313,14 @@ static int intel_hdmi_hdcp_write(struct intel_digital_port *dig_port,
 				 unsigned int offset, void *buffer, size_t size)
 {
 	struct intel_hdmi *hdmi = &dig_port->hdmi;
-	struct i2c_adapter *ddc = hdmi->attached_connector->base.ddc;
+	struct i2c_adapter *ddc = to_ddc(hdmi);
 	int ret;
 	u8 *write_buf;
 	struct i2c_msg msg;
 
+	if (!ddc)
+		return -EINVAL;
+
 	write_buf = kzalloc(size + 1, GFP_KERNEL);
 	if (!write_buf)
 		return -ENOMEM;
@@ -1335,9 +1349,12 @@ int intel_hdmi_hdcp_write_an_aksv(struct intel_digital_port *dig_port,
 {
 	struct intel_display *display = to_intel_display(dig_port);
 	struct intel_hdmi *hdmi = &dig_port->hdmi;
-	struct i2c_adapter *ddc = hdmi->attached_connector->base.ddc;
+	struct i2c_adapter *ddc = to_ddc(hdmi);
 	int ret;
 
+	if (!ddc)
+		return -EINVAL;
+
 	ret = intel_hdmi_hdcp_write(dig_port, DRM_HDCP_DDC_AN, an,
 				    DRM_HDCP_AN_LEN);
 	if (ret) {
-- 
2.47.0.163.g1226f6d8fa-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-11-15  1:55 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-31 10:51 [RFC][PATCH] drm: i915: do not NULL deref hdmi attached_connector Sergey Senozhatsky
2024-10-31 11:20 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2024-10-31 11:33 ` [RFC][PATCH] " Jani Nikula
2024-10-31 13:43   ` Sergey Senozhatsky
2024-10-31 12:10 ` ✓ Fi.CI.BAT: success for " Patchwork
2024-10-31 18:52 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-11-13  8:39 ` [RFC][PATCH] " Sergey Senozhatsky
2024-11-13  9:19   ` Jani Nikula
2024-11-14 15:53     ` Jani Nikula
2024-11-15  1:54       ` Sergey Senozhatsky

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).