drmres connector cleanup typically run after devres has released the
last dw-hdmi bridge reference. Since struct dw_hdmi, where the connector
lives, is freed when the last bridge reference is released, connector
cleanup can end up accessing freed memory.

Call trace without a bridge reference held until connector cleanup:
- dw_hdmi_bridge_detach()
- dw_hdmi_bridge_destroy()      <<-- struct dw_hdmi is free()
- [drm:drm_managed_release] drmres release begin
- [drm:drm_managed_release] REL (____ptrval____) drm_mode_config_init_release 
(0 bytes)
- dw_hdmi_connector_destroy()
  - drm_connector_cleanup()     <<-- drm_connector is use-after-free
  [...]
- [drm:drm_managed_release] drmres release end

Hold a bridge reference for as long as the connector exists and drop it
after drm_connector_cleanup() has completed to keep struct dw_hdmi alive
until connector teardown is finished and avoids the use-after-free.

Call trace with a bridge reference held until connector cleanup:
- dw_hdmi_bridge_detach()
- [drm:drm_managed_release] drmres release begin
- [drm:drm_managed_release] REL (____ptrval____) drm_mode_config_init_release 
(0 bytes)
- dw_hdmi_connector_destroy()
  - drm_connector_cleanup()     <<-- drm_connector is destroy()
  - drm_bridge_put()
- dw_hdmi_bridge_destroy()      <<-- struct dw_hdmi is free()
  [...]
- [drm:drm_managed_release] drmres release end

Signed-off-by: Jonas Karlman <[email protected]>
---
v5: New patch
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index a176eb55418c..cbbd15578042 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2528,10 +2528,18 @@ static void dw_hdmi_connector_force(struct 
drm_connector *connector)
        mutex_unlock(&hdmi->mutex);
 }
 
+static void dw_hdmi_connector_destroy(struct drm_connector *connector)
+{
+       struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi, 
connector);
+
+       drm_connector_cleanup(connector);
+       drm_bridge_put(&hdmi->bridge);
+}
+
 static const struct drm_connector_funcs dw_hdmi_connector_funcs = {
        .fill_modes = drm_helper_probe_single_connector_modes,
        .detect = dw_hdmi_connector_detect,
-       .destroy = drm_connector_cleanup,
+       .destroy = dw_hdmi_connector_destroy,
        .force = dw_hdmi_connector_force,
        .reset = drm_atomic_helper_connector_reset,
        .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
@@ -2548,6 +2556,7 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi)
        struct drm_connector *connector = &hdmi->connector;
        struct cec_connector_info conn_info;
        struct cec_notifier *notifier;
+       int ret;
 
        if (hdmi->version >= 0x200a)
                connector->ycbcr_420_allowed =
@@ -2560,10 +2569,14 @@ static int dw_hdmi_connector_create(struct dw_hdmi 
*hdmi)
 
        drm_connector_helper_add(connector, &dw_hdmi_connector_helper_funcs);
 
-       drm_connector_init_with_ddc(hdmi->bridge.dev, connector,
-                                   &dw_hdmi_connector_funcs,
-                                   DRM_MODE_CONNECTOR_HDMIA,
-                                   hdmi->ddc);
+       ret = drm_connector_init_with_ddc(hdmi->bridge.dev, connector,
+                                         &dw_hdmi_connector_funcs,
+                                         DRM_MODE_CONNECTOR_HDMIA,
+                                         hdmi->ddc);
+       if (ret)
+               return ret;
+
+       drm_bridge_get(&hdmi->bridge);
 
        /*
         * drm_connector_attach_max_bpc_property() requires the
-- 
2.54.0

Reply via email to