[bug report] drm: bridge: cdns-mhdp8546: Fix possible null pointer dereference

[Aleksandr Mishin]

On 11.04.2024 16:39, Dan Carpenter wrote:
> Hello Aleksandr Mishin,
> 
> Commit 935a92a1c400 ("drm: bridge: cdns-mhdp8546: Fix possible null
> pointer dereference") from Apr 8, 2024 (linux-next), leads to the
> following Smatch static checker warning:
> 
> 	drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c:2074 cdns_mhdp_atomic_enable()
> 	warn: inconsistent returns '&mhdp->link_mutex'.
> 
> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>      1986 static void cdns_mhdp_atomic_enable(struct drm_bridge *bridge,
>      1987                                     struct drm_bridge_state *bridge_state)
>      1988 {
>      1989         struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
>      1990         struct drm_atomic_state *state = bridge_state->base.state;
>      1991         struct cdns_mhdp_bridge_state *mhdp_state;
>      1992         struct drm_crtc_state *crtc_state;
>      1993         struct drm_connector *connector;
>      1994         struct drm_connector_state *conn_state;
>      1995         struct drm_bridge_state *new_state;
>      1996         const struct drm_display_mode *mode;
>      1997         u32 resp;
>      1998         int ret;
>      1999
>      2000         dev_dbg(mhdp->dev, "bridge enable\n");
>      2001
>      2002         mutex_lock(&mhdp->link_mutex);
>                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Holding a lock
> 
>      2003
>      2004         if (mhdp->plugged && !mhdp->link_up) {
>      2005                 ret = cdns_mhdp_link_up(mhdp);
>      2006                 if (ret < 0)
>      2007                         goto out;
>      2008         }
>      2009
>      2010         if (mhdp->info && mhdp->info->ops && mhdp->info->ops->enable)
>      2011                 mhdp->info->ops->enable(mhdp);
>      2012
>      2013         /* Enable VIF clock for stream 0 */
>      2014         ret = cdns_mhdp_reg_read(mhdp, CDNS_DPTX_CAR, &resp);
>      2015         if (ret < 0) {
>      2016                 dev_err(mhdp->dev, "Failed to read CDNS_DPTX_CAR %d\n", ret);
>      2017                 goto out;
>      2018         }
>      2019
>      2020         cdns_mhdp_reg_write(mhdp, CDNS_DPTX_CAR,
>      2021                             resp | CDNS_VIF_CLK_EN | CDNS_VIF_CLK_RSTN);
>      2022
>      2023         connector = drm_atomic_get_new_connector_for_encoder(state,
>      2024                                                              bridge->encoder);
>      2025         if (WARN_ON(!connector))
>      2026                 goto out;
>      2027
>      2028         conn_state = drm_atomic_get_new_connector_state(state, connector);
>      2029         if (WARN_ON(!conn_state))
>      2030                 goto out;
>      2031
>      2032         if (mhdp->hdcp_supported &&
>      2033             mhdp->hw_state == MHDP_HW_READY &&
>      2034             conn_state->content_protection ==
>      2035             DRM_MODE_CONTENT_PROTECTION_DESIRED) {
>      2036                 mutex_unlock(&mhdp->link_mutex);
>      2037                 cdns_mhdp_hdcp_enable(mhdp, conn_state->hdcp_content_type);
>      2038                 mutex_lock(&mhdp->link_mutex);
>      2039         }
>      2040
>      2041         crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
>      2042         if (WARN_ON(!crtc_state))
>      2043                 goto out;
>      2044
>      2045         mode = &crtc_state->adjusted_mode;
>      2046
>      2047         new_state = drm_atomic_get_new_bridge_state(state, bridge);
>      2048         if (WARN_ON(!new_state))
>      2049                 goto out;
>      2050
>      2051         if (!cdns_mhdp_bandwidth_ok(mhdp, mode, mhdp->link.num_lanes,
>      2052                                     mhdp->link.rate)) {
>      2053                 ret = -EINVAL;
>      2054                 goto out;
>      2055         }
>      2056
>      2057         cdns_mhdp_sst_enable(mhdp, mode);
>      2058
>      2059         mhdp_state = to_cdns_mhdp_bridge_state(new_state);
>      2060
>      2061         mhdp_state->current_mode = drm_mode_duplicate(bridge->dev, mode);
>      2062         if (!mhdp_state->current_mode)
>      2063                 return;
>                           ^^^^^^^
> Needs to unlock before returning.

Yes. Sorry. It's my mistake.
I'll replace 'return' with 'ret=-EINVAL; goto out;' and offer v2 patch.

> 
>      2064
>      2065         drm_mode_set_name(mhdp_state->current_mode);
>      2066
>      2067         dev_dbg(mhdp->dev, "%s: Enabling mode %s\n", __func__, mode->name);
>      2068
>      2069         mhdp->bridge_enabled = true;
>      2070
>      2071 out:
>      2072         mutex_unlock(&mhdp->link_mutex);
>      2073         if (ret < 0)
> --> 2074                 schedule_work(&mhdp->modeset_retry_work);
>      2075 }
> 
> regards,
> dan carpenter

-- 
Kind regards
Aleksandr