From: Aleksandr Mishin <amishin@t-argos.ru>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: <dri-devel@lists.freedesktop.org>
Subject: Re: [bug report] drm: bridge: cdns-mhdp8546: Fix possible null pointer dereference
Date: Thu, 11 Apr 2024 16:49:57 +0300 [thread overview]
Message-ID: <d6b7a51e-216e-4059-9f28-74fc47078ecf@t-argos.ru> (raw)
In-Reply-To: <a6e7e2f3-c173-4c33-ba0b-b068140131a9@moroto.mountain>
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
prev parent reply other threads:[~2024-04-11 13:53 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d6b7a51e-216e-4059-9f28-74fc47078ecf@t-argos.ru \
--to=amishin@t-argos.ru \
--cc=dan.carpenter@linaro.org \
--cc=dri-devel@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.