linux-kernel - [PATCH] drm/dp/mst: Validate port in drm_dp_payload_send

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

Date:	Mon,  4 Apr 2016 19:33:46 -0400
From:	Lyude <cpaul@...hat.com>
To:	dri-devel@...ts.freedesktop.org
Cc:	Lyude <cpaul@...hat.com>, David Airlie <airlied@...ux.ie>,
	linux-kernel@...r.kernel.org (open list)
Subject: [PATCH] drm/dp/mst: Validate port in drm_dp_payload_send_msg()

With the joys of things running concurrently, there's always a chance
that the port we get passed in drm_dp_payload_send_msg() isn't actually
valid anymore. Because of this, we need to make sure we validate the
reference to the port before we use it otherwise we risk running into
various race conditions. For instance, on the Dell MST monitor I have
here for testing, hotplugging it enough times causes us to kernel panic:

[drm:intel_mst_enable_dp] 1
[drm:drm_dp_update_payload_part2] payload 0 1
[drm:intel_get_hpd_pins] hotplug event received, stat 0x00200000, dig 0x10101011, pins 0x00000020
[drm:intel_hpd_irq_handler] digital hpd port B - short
[drm:intel_dp_hpd_pulse] got hpd irq on port B - short
[drm:intel_dp_check_mst_status] got esi 00 10 00
[drm:drm_dp_update_payload_part2] payload 1 1
general protection fault: 0000 [#1] SMP
…
Call Trace:
 [<ffffffffa012b632>] drm_dp_update_payload_part2+0xc2/0x130 [drm_kms_helper]
 [<ffffffffa032ef08>] intel_mst_enable_dp+0xf8/0x180 [i915]
 [<ffffffffa0310dbd>] haswell_crtc_enable+0x3ed/0x8c0 [i915]
 [<ffffffffa030c84d>] intel_atomic_commit+0x5ad/0x1590 [i915]
 [<ffffffffa01db877>] ? drm_atomic_set_crtc_for_connector+0x57/0xe0 [drm]
 [<ffffffffa01dc4e7>] drm_atomic_commit+0x37/0x60 [drm]
 [<ffffffffa0130a3a>] drm_atomic_helper_set_config+0x7a/0xb0 [drm_kms_helper]
 [<ffffffffa01cc482>] drm_mode_set_config_internal+0x62/0x100 [drm]
 [<ffffffffa01d02ad>] drm_mode_setcrtc+0x3cd/0x4e0 [drm]
 [<ffffffffa01c18e3>] drm_ioctl+0x143/0x510 [drm]
 [<ffffffffa01cfee0>] ? drm_mode_setplane+0x1b0/0x1b0 [drm]
 [<ffffffff810f79a7>] ? hrtimer_start_range_ns+0x1b7/0x3a0
 [<ffffffff81212962>] do_vfs_ioctl+0x92/0x570
 [<ffffffff81590852>] ? __sys_recvmsg+0x42/0x80
 [<ffffffff81212eb9>] SyS_ioctl+0x79/0x90
 [<ffffffff816b4e32>] entry_SYSCALL_64_fastpath+0x1a/0xa4
RIP  [<ffffffffa012b026>] drm_dp_payload_send_msg+0x146/0x1f0 [drm_kms_helper]

Which occurs because of the hotplug event shown in the log, which ends
up causing DRM's dp helpers to drop the port we're updating the payload
on and panic.

Signed-off-by: Lyude <cpaul@...hat.com>
Reviewed-by: David Airlie <airlied@...ux.ie>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 27fbd79..e17fbda 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1672,13 +1672,19 @@ static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
 	u8 sinks[DRM_DP_MAX_SDP_STREAMS];
 	int i;
 
+	port = drm_dp_get_validated_port_ref(mgr, port);
+	if (!port)
+		return -EINVAL;
+
 	port_num = port->port_num;
 	mstb = drm_dp_get_validated_mstb_ref(mgr, port->parent);
 	if (!mstb) {
 		mstb = drm_dp_get_last_connected_port_and_mstb(mgr, port->parent, &port_num);
 
-		if (!mstb)
+		if (!mstb) {
+			drm_dp_put_port(port);
 			return -EINVAL;
+		}
 	}
 
 	txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
@@ -1707,6 +1713,7 @@ static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
 	kfree(txmsg);
 fail_put:
 	drm_dp_put_mst_branch_device(mstb);
+	drm_dp_put_port(port);
 	return ret;
 }
 
-- 
2.5.5