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 (...) 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 (...) 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
Fixes: ed6987b67418 ("drm/bridge: dw-hdmi: convert to devm_drm_bridge_alloc()
API")
Tested-by: Diederik de Haas <[email protected]> # Rock64, RockPro64,
Quartz64-B
Signed-off-by: Jonas Karlman <[email protected]>
---
v7: Add fixes tag, re-order patch
v6: Collect t-b tag
v5: New patch
This use-after-free issue likely existed before commit ed6987b67418 when
devm_kzalloc() was used instead of devm_drm_bridge_alloc(). However,
v6.16-rc1 first introduced bridge refcount and drm_bridge_put(),
parts that are used to help fix the use-after-free issue.
KASAN report a slab-use-after-free in __refcount_add_not_zero when,
echo fe0a0000.hdmi > /sys/bus/platform/drivers/dwhdmi-rockchip/unbind
on a Rockchip RK3566 device prior to this fix.
---
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 b7bfc0e9a6b2..9d795c550f8a 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2568,10 +2568,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,
@@ -2588,6 +2596,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 =
@@ -2600,10 +2609,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
