[bug report] drm/msm/dsi: Split PHY drivers to separate files

[bug report] drm/msm/dsi: Split PHY drivers to separate files

[Dan Carpenter]

Hello Hai Li,

This is a semi-automatic email about new static checker warnings.

The patch 5c8290284402: "drm/msm/dsi: Split PHY drivers to separate 
files" from Aug 13, 2015, leads to the following Smatch complaint:

    drivers/gpu/drm/msm/dsi/phy/dsi_phy.c:812 msm_dsi_phy_enable()
    warn: variable dereferenced before check 'phy' (see line 809)

drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
   805  int msm_dsi_phy_enable(struct msm_dsi_phy *phy,
   806                          struct msm_dsi_phy_clk_request *clk_req,
   807                          struct msm_dsi_phy_shared_timings *shared_timings)
   808	{
   809		struct device *dev = &phy->pdev->dev;
                                      ^^^^^^^^^^^^^^
"phy" is deferenced without checking

   810		int ret;
   811	
   812		if (!phy || !phy->cfg->ops.enable)
                     ^^^
Too late.  The fact that no one has complained in six years suggests
that the NULL check can be removed but it wasn't 100% to obvious from
reading the code.

   813			return -EINVAL;
   814	

regards,
dan carpenter

Re: [bug report] drm/msm/dsi: Split PHY drivers to separate files

[Dmitry Baryshkov]

On 04/10/2021 13:37, Dan Carpenter wrote:
> Hello Hai Li,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 5c8290284402: "drm/msm/dsi: Split PHY drivers to separate
> files" from Aug 13, 2015, leads to the following Smatch complaint:
> 
>      drivers/gpu/drm/msm/dsi/phy/dsi_phy.c:812 msm_dsi_phy_enable()
>      warn: variable dereferenced before check 'phy' (see line 809)
> 
> drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
>     805  int msm_dsi_phy_enable(struct msm_dsi_phy *phy,
>     806                          struct msm_dsi_phy_clk_request *clk_req,
>     807                          struct msm_dsi_phy_shared_timings *shared_timings)
>     808	{
>     809		struct device *dev = &phy->pdev->dev;
>                                        ^^^^^^^^^^^^^^
> "phy" is deferenced without checking
> 
>     810		int ret;
>     811	
>     812		if (!phy || !phy->cfg->ops.enable)
>                       ^^^
> Too late.  The fact that no one has complained in six years suggests
> that the NULL check can be removed but it wasn't 100% to obvious from
> reading the code.

Yes, phy can not be NULL, since otherwise dsi_get_phy would make 
dsi_init return -EPROBE_DEFER.

> 
>     813			return -EINVAL;
>     814	
> 
> regards,
> dan carpenter
> 


-- 
With best wishes
Dmitry

Re: Fwd: [bug report] drm/msm/dsi: Split PHY drivers to separate files

[jesszhan]

Hey Dan,

On 2021-10-04 16:28, abhinavk@codeaurora.org wrote:
> -------- Original Message --------
> Subject: [bug report] drm/msm/dsi: Split PHY drivers to separate files
> Date: 2021-10-04 03:37
> From: Dan Carpenter <dan.carpenter@oracle.com>
> To: hali@codeaurora.org
> Cc: linux-arm-msm@vger.kernel.org
> 
> Hello Hai Li,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 5c8290284402: "drm/msm/dsi: Split PHY drivers to separate
> files" from Aug 13, 2015, leads to the following Smatch complaint:
> 
>     drivers/gpu/drm/msm/dsi/phy/dsi_phy.c:812 msm_dsi_phy_enable()
>     warn: variable dereferenced before check 'phy' (see line 809)
> 
> drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
>    805  int msm_dsi_phy_enable(struct msm_dsi_phy *phy,
>    806                          struct msm_dsi_phy_clk_request 
> *clk_req,
>    807                          struct msm_dsi_phy_shared_timings
> *shared_timings)
>    808	{
>    809		struct device *dev = &phy->pdev->dev;
>                                       ^^^^^^^^^^^^^^
> "phy" is deferenced without checking
> 
>    810		int ret;
>    811
>    812		if (!phy || !phy->cfg->ops.enable)
>                      ^^^
> Too late.  The fact that no one has complained in six years suggests
> that the NULL check can be removed but it wasn't 100% to obvious from
> reading the code.
> 
>    813			return -EINVAL;
>    814
> 
> regards,
> dan carpenter

Thanks for the report, will look into it.

Best,
Jessica Zhang