[PATCH v2] drm/msm: Fix incorrect file name output in adreno_request_fw()
In adreno_request_fw() when debugging information is printed to the log after firmware load, an incorrect filename is printed. 'newname' is used instead of 'fwname', so prefix "qcom/" is being added to filename. Looks like "copy-paste" mistake. Fix this mistake by replacing 'newname' with 'fwname'. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 2c41ef1b6f7d ("drm/msm/adreno: deal with linux-firmware fw paths") Signed-off-by: Aleksandr Mishin --- v1->v2: Fix incorrect 'Fixes' tag drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 074fb498706f..0bb7d66047f8 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -475,7 +475,7 @@ adreno_request_fw(struct adreno_gpu *adreno_gpu, const char *fwname) ret = request_firmware_direct(, fwname, drm->dev); if (!ret) { DRM_DEV_INFO(drm->dev, "loaded %s from legacy location\n", - newname); + fwname); adreno_gpu->fwloc = FW_LOCATION_LEGACY; goto out; } else if (adreno_gpu->fwloc != FW_LOCATION_UNKNOWN) { -- 2.30.2
[PATCH] drm/msm: Fix incorrect file name output in adreno_request_fw()
In adreno_request_fw() when debugging information is printed to the log after firmware load, an incorrect filename is printed. 'newname' is used instead of 'fwname', so prefix "qcom/" is being added to filename. Looks like "copy-paste" mistake. Fix this mistake by replacing 'newname' with 'fwname'. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 9fe041f6fdfe ("drm/msm: Add msm_gem_get_and_pin_iova()") Signed-off-by: Aleksandr Mishin --- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 074fb498706f..0bb7d66047f8 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -475,7 +475,7 @@ adreno_request_fw(struct adreno_gpu *adreno_gpu, const char *fwname) ret = request_firmware_direct(, fwname, drm->dev); if (!ret) { DRM_DEV_INFO(drm->dev, "loaded %s from legacy location\n", - newname); + fwname); adreno_gpu->fwloc = FW_LOCATION_LEGACY; goto out; } else if (adreno_gpu->fwloc != FW_LOCATION_UNKNOWN) { -- 2.30.2
[PATCH] drm: bridge: cdns-mhdp8546: Fix missing mutex unlock on error path
In cdns_mhdp_atomic_enable(), there is an error return on failure of drm_mode_duplicate() which leads to the mutex remaining locked. Add a mutex unlock call. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 935a92a1c400 ("drm: bridge: cdns-mhdp8546: Fix possible null pointer dereference") Signed-off-by: Aleksandr Mishin --- This patch is against drm-misc-next branch of drm-misc repo. drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index 8a91ef0ae065..65a4bd09d9c6 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -2059,8 +2059,10 @@ static void cdns_mhdp_atomic_enable(struct drm_bridge *bridge, mhdp_state = to_cdns_mhdp_bridge_state(new_state); mhdp_state->current_mode = drm_mode_duplicate(bridge->dev, mode); - if (!mhdp_state->current_mode) - return; + if (!mhdp_state->current_mode) { + ret = -EINVAL; + goto out; + } drm_mode_set_name(mhdp_state->current_mode); -- 2.30.2
[PATCH v2] drm: bridge: cdns-mhdp8546: Fix possible null pointer dereference
In cdns_mhdp_atomic_enable(), the return value of drm_mode_duplicate() is assigned to mhdp_state->current_mode, and there is a dereference of it in drm_mode_set_name(), which will lead to a NULL pointer dereference on failure of drm_mode_duplicate(). Fix this bug by adding a check of mhdp_state->current_mode. Fixes: fb43aa0acdfd ("drm: bridge: Add support for Cadence MHDP8546 DPI/DP bridge") Signed-off-by: Aleksandr Mishin --- v2: Fix a mistake where the mutex remained locked drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index e226acc5c15e..5b831d6d7764 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -2059,6 +2059,11 @@ static void cdns_mhdp_atomic_enable(struct drm_bridge *bridge, mhdp_state = to_cdns_mhdp_bridge_state(new_state); mhdp_state->current_mode = drm_mode_duplicate(bridge->dev, mode); + if (!mhdp_state->current_mode) { + ret = -EINVAL; + goto out; + } + drm_mode_set_name(mhdp_state->current_mode); dev_dbg(mhdp->dev, "%s: Enabling mode %s\n", __func__, mode->name); -- 2.30.2
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
Re: [PATCH] drm/msm/dpu: Add callback function pointer check before its call
On 08.04.2024 12:03, Dmitry Baryshkov wrote: On Mon, 8 Apr 2024 at 11:57, Aleksandr Mishin wrote: In dpu_core_irq_callback_handler() callback function pointer is compared to NULL, but then callback function is unconditionally called by this pointer. Fix this bug by adding conditional return. Found by Linux Verification Center (linuxtesting.org) with SVACE. This should be converted to a proper Reported-by: trailer. It is an established practice for our project, you can find 700+ applied patches with similar line: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?qt=grep=linuxtesting.org Fixes: c929ac60b3ed ("drm/msm/dpu: allow just single IRQ callback") Signed-off-by: Aleksandr Mishin --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c index 946dd0135dff..03a16fbd4c99 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c @@ -223,9 +223,11 @@ static void dpu_core_irq_callback_handler(struct dpu_kms *dpu_kms, unsigned int VERB("IRQ=[%d, %d]\n", DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx)); - if (!irq_entry->cb) + if (!irq_entry->cb) { DRM_ERROR("no registered cb, IRQ=[%d, %d]\n", DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx)); + return; + } atomic_inc(_entry->count); -- 2.30.2 -- Kind regards Aleksandr
Re: [PATCH] drm/msm/dpu: Add callback function pointer check before its call
On 08.04.2024 19:51, Abhinav Kumar wrote: On 4/8/2024 1:55 AM, Aleksandr Mishin wrote: In dpu_core_irq_callback_handler() callback function pointer is compared to NULL, but then callback function is unconditionally called by this pointer. Fix this bug by adding conditional return. Found by Linux Verification Center (linuxtesting.org) with SVACE. Yes , as dmitry wrote, this should be Reported-by. It is an established practice for our project, you can find 700+ applied patches with similar line: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?qt=grep=linuxtesting.org But rest LGTM. Fixes: c929ac60b3ed ("drm/msm/dpu: allow just single IRQ callback") Signed-off-by: Aleksandr Mishin --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c index 946dd0135dff..03a16fbd4c99 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c @@ -223,9 +223,11 @@ static void dpu_core_irq_callback_handler(struct dpu_kms *dpu_kms, unsigned int VERB("IRQ=[%d, %d]\n", DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx)); - if (!irq_entry->cb) + if (!irq_entry->cb) { DRM_ERROR("no registered cb, IRQ=[%d, %d]\n", DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx)); + return; + } atomic_inc(_entry->count); -- Kind regards Aleksandr
[PATCH] drm: vc4: Fix possible null pointer dereference
In vc4_hdmi_audio_init() of_get_address() may return NULL which is later dereferenced. Fix this bug by adding NULL check. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: bb7d78568814 ("drm/vc4: Add HDMI audio support") Signed-off-by: Aleksandr Mishin --- drivers/gpu/drm/vc4/vc4_hdmi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index d8751ea20303..5f8d51b29370 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2740,6 +2740,8 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi) index = 1; addr = of_get_address(dev->of_node, index, NULL, NULL); + if (!addr) + return -EINVAL; vc4_hdmi->audio.dma_data.addr = be32_to_cpup(addr) + mai_data->offset; vc4_hdmi->audio.dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; -- 2.30.2
[PATCH] drm/msm/dpu: Add callback function pointer check before its call
In dpu_core_irq_callback_handler() callback function pointer is compared to NULL, but then callback function is unconditionally called by this pointer. Fix this bug by adding conditional return. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: c929ac60b3ed ("drm/msm/dpu: allow just single IRQ callback") Signed-off-by: Aleksandr Mishin --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c index 946dd0135dff..03a16fbd4c99 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c @@ -223,9 +223,11 @@ static void dpu_core_irq_callback_handler(struct dpu_kms *dpu_kms, unsigned int VERB("IRQ=[%d, %d]\n", DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx)); - if (!irq_entry->cb) + if (!irq_entry->cb) { DRM_ERROR("no registered cb, IRQ=[%d, %d]\n", DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx)); + return; + } atomic_inc(_entry->count); -- 2.30.2
[PATCH] drm: bridge: cdns-mhdp8546: Fix possible null pointer dereference
In cdns_mhdp_atomic_enable(), the return value of drm_mode_duplicate() is assigned to mhdp_state->current_mode, and there is a dereference of it in drm_mode_set_name(), which will lead to a NULL pointer dereference on failure of drm_mode_duplicate(). Fix this bug add a check of mhdp_state->current_mode. Fixes: fb43aa0acdfd ("drm: bridge: Add support for Cadence MHDP8546 DPI/DP bridge") Signed-off-by: Aleksandr Mishin --- drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index e226acc5c15e..8a91ef0ae065 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -2059,6 +2059,9 @@ static void cdns_mhdp_atomic_enable(struct drm_bridge *bridge, mhdp_state = to_cdns_mhdp_bridge_state(new_state); mhdp_state->current_mode = drm_mode_duplicate(bridge->dev, mode); + if (!mhdp_state->current_mode) + return; + drm_mode_set_name(mhdp_state->current_mode); dev_dbg(mhdp->dev, "%s: Enabling mode %s\n", __func__, mode->name); -- 2.30.2