Re: [bug report] drm: bridge: cdns-mhdp8546: Fix possible null pointer dereference
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 '>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(>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, );
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(>link_mutex);
2037 cdns_mhdp_hdcp_enable(mhdp,
conn_state->hdcp_content_type);
2038 mutex_lock(>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 = _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(>link_mutex);
2073 if (ret < 0)
--> 2074 schedule_work(>modeset_retry_work);
2075 }
regards,
dan carpenter
--
Kind regards
Aleksandr
[bug report] drm: bridge: cdns-mhdp8546: Fix possible null pointer dereference
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 '>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(>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, );
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(>link_mutex);
2037 cdns_mhdp_hdcp_enable(mhdp,
conn_state->hdcp_content_type);
2038 mutex_lock(>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 = _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.
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(>link_mutex);
2073 if (ret < 0)
--> 2074 schedule_work(>modeset_retry_work);
2075 }
regards,
dan carpenter
