drm/dp/mst: fix kernel oops when turning off secondary monitor

plbossart · jnikula · commit bb08c04dc867 · 2017-02-15T11:41:19.000+02:00
100% reproducible issue found on SKL SkullCanyon NUC with two external DP daisy-chained monitors in DP/MST mode. When turning off or changing the input of the second monitor the machine stops with a kernel oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly. This issue is traced to an inconsistent control flow in drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at the same time as 'req_payload.num_slots' is set to zero, but the pointer is dereferenced even when req_payload.num_slot is zero. The problematic dereference was introduced in commit dfda0df ("drm/mst: rework payload table allocation to conform better") and may impact all versions since v3.18 The fix suggested by Chris Wilson removes the kernel oops and was found to work well after 10mn of monkey-testing with the second monitor power and input buttons Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990 Fixes: dfda0df ("drm/mst: rework payload table allocation to conform better.") Cc: Dave Airlie <airlied@redhat.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Nathan D Ciobanu <nathan.d.ciobanu@linux.intel.com> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Cc: Sean Paul <seanpaul@chromium.org> Cc: <stable@vger.kernel.org> # v3.18+ Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@linux.intel.com> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1487076561-2169-1-git-send-email-jani.nikula@intel.com
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
 				mgr->payloads[i].vcpi = req_payload.vcpi;
 			} else if (mgr->payloads[i].num_slots) {
 				mgr->payloads[i].num_slots = 0;
-				drm_dp_destroy_payload_step1(mgr, port, port->vcpi.vcpi, &mgr->payloads[i]);
+				drm_dp_destroy_payload_step1(mgr, port, mgr->payloads[i].vcpi, &mgr->payloads[i]);
 				req_payload.payload_state = mgr->payloads[i].payload_state;
 				mgr->payloads[i].start_slot = 0;
 			}

Original file line number	Diff line number	Diff line change
`@@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)`
`1817`	`1817`	`mgr->payloads[i].vcpi = req_payload.vcpi;`
`1818`	`1818`	`} else if (mgr->payloads[i].num_slots) {`
`1819`	`1819`	`mgr->payloads[i].num_slots = 0;`
`1820`		`- drm_dp_destroy_payload_step1(mgr, port, port->vcpi.vcpi, &mgr->payloads[i]);`
	`1820`	`+ drm_dp_destroy_payload_step1(mgr, port, mgr->payloads[i].vcpi, &mgr->payloads[i]);`
`1821`	`1821`	`req_payload.payload_state = mgr->payloads[i].payload_state;`
`1822`	`1822`	`mgr->payloads[i].start_slot = 0;`
`1823`	`1823`	`}`