* [bug report] drm: bridge: cdns-mhdp8546: Fix possible null pointer dereference
@ 2024-04-11 13:39 Dan Carpenter
2024-04-11 13:49 ` Aleksandr Mishin
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2024-04-11 13:39 UTC (permalink / raw)
To: amishin; +Cc: dri-devel
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.
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] drm: bridge: cdns-mhdp8546: Fix possible null pointer dereference
2024-04-11 13:39 [bug report] drm: bridge: cdns-mhdp8546: Fix possible null pointer dereference Dan Carpenter
@ 2024-04-11 13:49 ` Aleksandr Mishin
0 siblings, 0 replies; 2+ messages in thread
From: Aleksandr Mishin @ 2024-04-11 13:49 UTC (permalink / raw)
To: Dan Carpenter; +Cc: dri-devel
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-04-11 13:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-11 13:39 [bug report] drm: bridge: cdns-mhdp8546: Fix possible null pointer dereference Dan Carpenter
2024-04-11 13:49 ` Aleksandr Mishin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).