⬢ DragonFlyBSD Kernel Audit
DF-5218 / fix.diff
← back to finding ↓ download raw
--- sys/dev/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ sys/dev/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -750,10 +750,17 @@ static void emulated_link_detect(struct dc_link *link)
 	link->type = dc_connection_none;
 	prev_sink = link->local_sink;
 
-	if (prev_sink != NULL)
-		dc_sink_retain(prev_sink);
+	if (prev_sink != NULL) {
+		dc_sink_retain(prev_sink);
+		/* Drop the LINK's reference now that prev_sink holds a
+		 * temporary one, so the local_sink store below cannot
+		 * strand the old sink's reference (permanent leak of a
+		 * struct dc_sink per forced-detect cycle).  Mirrors
+		 * link_disconnect_sink() in dc_link.c:492. */
+		dc_sink_release(link->local_sink);
+		link->local_sink = NULL;
+	}
 
 	switch (link->connector_signal) {
@@ -795,7 +802,7 @@ static void emulated_link_detect(struct dc_link *link)
 	default:
 		DC_ERROR("Invalid connector type! signal:%d\n",
 			link->connector_signal);
-		return;
+		goto out;
 	}
 
 	sink_init_data.link = link;
@@ -804,7 +811,7 @@ static void emulated_link_detect(struct dc_link *link)
 	sink = dc_sink_create(&sink_init_data);
 	if (!sink) {
 		DC_ERROR("Failed to create sink!\n");
-		return;
+		goto out;
 	}
 
 	link->local_sink = sink;
@@ -818,6 +825,10 @@ static void emulated_link_detect(struct dc_link *link)
 	if (edid_status != EDID_OK)
 		DC_ERROR("Failed to read EDID");
 
+out:
+	if (prev_sink != NULL)
+		dc_sink_release(prev_sink);
 }
 
 static int dm_resume(void *handle)
@@ -982,13 +993,10 @@
 		if (sink) {
 			if (aconnector->dc_sink) {
 				amdgpu_dm_update_freesync_caps(connector, NULL);
-				/*
-				 * retain and release below are used to
-				 * bump up refcount for sink because the link doesn't point
-				 * to it anymore after disconnect, so on next crtc to connector
-				 * reshuffle by UMD we will get into unwanted dc_sink release
-				 */
-				if (aconnector->dc_sink != aconnector->dc_em_sink)
-					dc_sink_release(aconnector->dc_sink);
+				/* aconnector->dc_sink is a BORROWED pointer (it is
+				 * assigned without a matching retain at :1001,
+				 * :1044 and amdgpu_dm.c:3278).  Do NOT release it
+				 * here: the dropped reference belongs to
+				 * dc_link->local_sink (still-live sink ->
+				 * refcount undercount, premature free) or was
+				 * already dropped by dc_link_detect()
+				 * (EDID-change cycle -> dc_sink_release() on
+				 * freed memory: kref_put UAF write and
+				 * dc_sink_free() double free). */
 			}
 			aconnector->dc_sink = sink;
 			amdgpu_dm_update_freesync_caps(connector,
@@ -998,8 +1006,11 @@
 		} else {
 			amdgpu_dm_update_freesync_caps(connector, NULL);
 			if (!aconnector->dc_sink)
 				aconnector->dc_sink = aconnector->dc_em_sink;
-			else if (aconnector->dc_sink != aconnector->dc_em_sink)
-				dc_sink_retain(aconnector->dc_sink);
+			/* The retain removed above was the counterpart of
+			 * the removed release; keeping it would leak one
+			 * reference per disconnect cycle.  aconnector->dc_sink
+			 * stays borrowed from dc_link->local_sink, exactly as
+			 * in the normal (non-forced) path below. */
 		}
 
 		mutex_unlock(&dev->mode_config.mutex);