linux-kernel - [PATCH 3/3] drm/dp_mst: Fix topology ref leak in drm_dp_mst_allocate

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

Date:   Wed, 16 Jan 2019 15:37:59 -0500
From:   Lyude Paul <lyude@...hat.com>
To:     dri-devel@...ts.freedesktop.org
Cc:     David Airlie <airlied@...ux.ie>, Daniel Vetter <daniel@...ll.ch>,
        stable@...r.kernel.org,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <maxime.ripard@...tlin.com>,
        Sean Paul <sean@...rly.run>, linux-kernel@...r.kernel.org
Subject: [PATCH 3/3] drm/dp_mst: Fix topology ref leak in drm_dp_mst_allocate_vcpi()

Another drive-by fix. If slots < 0, or drm_dp_init_vcpi() fails, we'll
end up returning from the function without dropping the topology ref
that we grabbed at the very start. This would result in an MST hub
and/or it's ports staying around even after the MST topology has been
removed from the system.

So, fix this by making sure that we always drop the topology ref to port
when returning from this function.

Additionally: it looks like this bug exists pre-topology & malloc krefs,
so let's also make sure this gets backported to stable.

Signed-off-by: Lyude Paul <lyude@...hat.com>
Fixes: ad7f8a1f9ced ("drm/helper: add Displayport multi-stream helper (v0.6)")
Cc: David Airlie <airlied@...ux.ie>
Cc: Daniel Vetter <daniel@...ll.ch>
Cc: <stable@...r.kernel.org> # v3.17+
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index d99560c5c693..403f035dc8b8 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3177,28 +3177,29 @@ EXPORT_SYMBOL(drm_dp_atomic_release_vcpi_slots);
 bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
 			      struct drm_dp_mst_port *port, int pbn, int slots)
 {
-	int ret;
+	int rc;
+	bool ret = false;
 
-	port = drm_dp_mst_topology_get_port_validated(mgr, port);
-	if (!port)
+	if (slots < 0)
 		return false;
 
-	if (slots < 0)
+	port = drm_dp_mst_topology_get_port_validated(mgr, port);
+	if (!port)
 		return false;
 
 	if (port->vcpi.vcpi > 0) {
 		DRM_DEBUG_KMS("payload: vcpi %d already allocated for pbn %d - requested pbn %d\n",
 			      port->vcpi.vcpi, port->vcpi.pbn, pbn);
 		if (pbn == port->vcpi.pbn) {
-			drm_dp_mst_topology_put_port(port);
-			return true;
+			ret = true;
+			goto out;
 		}
 	}
 
-	ret = drm_dp_init_vcpi(mgr, &port->vcpi, pbn, slots);
-	if (ret) {
+	rc = drm_dp_init_vcpi(mgr, &port->vcpi, pbn, slots);
+	if (rc) {
 		DRM_DEBUG_KMS("failed to init vcpi slots=%d max=63 ret=%d\n",
-			      DIV_ROUND_UP(pbn, mgr->pbn_div), ret);
+			      DIV_ROUND_UP(pbn, mgr->pbn_div), rc);
 		goto out;
 	}
 	DRM_DEBUG_KMS("initing vcpi for pbn=%d slots=%d\n",
@@ -3206,10 +3207,10 @@ bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
 
 	/* Keep port allocated until it's payload has been removed */
 	drm_dp_mst_get_port_malloc(port);
-	drm_dp_mst_topology_put_port(port);
-	return true;
+	ret = true;
 out:
-	return false;
+	drm_dp_mst_topology_put_port(port);
+	return ret;
 }
 EXPORT_SYMBOL(drm_dp_mst_allocate_vcpi);
 
-- 
2.20.1