Re: [Intel-gfx] [PATCH] drm/dp/mst: Remove port after removing connector.

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Aug 11, 2015 at 09:54:29AM +0200, Maarten Lankhorst wrote:
> The port is removed synchronously, but the connector delayed.
> This causes a use after free which can cause a kernel BUG with
> slug_debug=FPZU. This is fixed by freeing the port after the
> connector.
> 
> This fixes a regression introduced with
> 6b8eeca65b18ae77e175cc2b6571731f0ee413bf
> "drm/dp/mst: close deadlock in connector destruction."
> 
> Cc: stable@vger.kernel.org
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Jani, can you please pick this up for topic/drm-fixes since Dave's still
on vacation this week?
-Daniel

> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 19 +++++++++++++------
>  include/drm/drm_crtc.h                |  2 --
>  2 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index b0487c9f018c..eb603f1defc2 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -873,9 +873,10 @@ static void drm_dp_destroy_port(struct kref *kref)
>  		   from an EDID retrieval */
>  		if (port->connector) {
>  			mutex_lock(&mgr->destroy_connector_lock);
> -			list_add(&port->connector->destroy_list, &mgr->destroy_connector_list);
> +			list_add(&port->next, &mgr->destroy_connector_list);
>  			mutex_unlock(&mgr->destroy_connector_lock);
>  			schedule_work(&mgr->destroy_connector_work);
> +			return;
>  		}
>  		drm_dp_port_teardown_pdt(port, port->pdt);
>  
> @@ -2659,7 +2660,7 @@ static void drm_dp_tx_work(struct work_struct *work)
>  static void drm_dp_destroy_connector_work(struct work_struct *work)
>  {
>  	struct drm_dp_mst_topology_mgr *mgr = container_of(work, struct drm_dp_mst_topology_mgr, destroy_connector_work);
> -	struct drm_connector *connector;
> +	struct drm_dp_mst_port *port;
>  
>  	/*
>  	 * Not a regular list traverse as we have to drop the destroy
> @@ -2668,15 +2669,21 @@ static void drm_dp_destroy_connector_work(struct work_struct *work)
>  	 */
>  	for (;;) {
>  		mutex_lock(&mgr->destroy_connector_lock);
> -		connector = list_first_entry_or_null(&mgr->destroy_connector_list, struct drm_connector, destroy_list);
> -		if (!connector) {
> +		port = list_first_entry_or_null(&mgr->destroy_connector_list, struct drm_dp_mst_port, next);
> +		if (!port) {
>  			mutex_unlock(&mgr->destroy_connector_lock);
>  			break;
>  		}
> -		list_del(&connector->destroy_list);
> +		list_del(&port->next);
>  		mutex_unlock(&mgr->destroy_connector_lock);
>  
> -		mgr->cbs->destroy_connector(mgr, connector);
> +		mgr->cbs->destroy_connector(mgr, port->connector);
> +
> +		drm_dp_port_teardown_pdt(port, port->pdt);
> +
> +		if (!port->input && port->vcpi.vcpi > 0)
> +			drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
> +		kfree(port);
>  	}
>  }
>  
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 574656965126..373b1bc6de96 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -745,8 +745,6 @@ struct drm_connector {
>  	uint8_t num_h_tile, num_v_tile;
>  	uint8_t tile_h_loc, tile_v_loc;
>  	uint16_t tile_h_size, tile_v_size;
> -
> -	struct list_head destroy_list;
>  };
>  
>  /**
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch