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