netdev - Re: [PATCH net-next 1/1] drivers/phylib: fix coverity issue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 17 Jan 2023 15:14:02 -0800
From:   Jacob Keller <jacob.e.keller@...el.com>
To:     Piergiorgio Beruto <piergiorgio.beruto@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        "Eric Dumazet" <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>
CC:     <linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>,
        Oleksij Rempel <o.rempel@...gutronix.de>,
        <mailhol.vincent@...adoo.fr>, <sudheer.mogilappagari@...el.com>,
        <sbhatta@...vell.com>, <linux-doc@...r.kernel.org>,
        <wangjie125@...wei.com>, <corbet@....net>, <lkp@...el.com>,
        <gal@...dia.com>, <gustavoars@...nel.org>, <bagasdotme@...il.com>
Subject: Re: [PATCH net-next 1/1] drivers/phylib: fix coverity issue



On 1/17/2023 1:47 PM, Piergiorgio Beruto wrote:
> Coverity reported the following:
> 
> *** CID 1530573:    (UNINIT)
> drivers/net/phy/phy-c45.c:1036 in genphy_c45_plca_set_cfg()
> 1030     				return ret;
> 1031
> 1032     			val = ret;
> 1033     		}
> 1034
> 1035     		if (plca_cfg->node_cnt >= 0)
> vvv     CID 1530573:    (UNINIT)
> vvv     Using uninitialized value "val".
> 1036     			val = (val & ~MDIO_OATC14_PLCA_NCNT) |
> 1037     			      (plca_cfg->node_cnt << 8);
> 1038
> 1039     		if (plca_cfg->node_id >= 0)
> 1040     			val = (val & ~MDIO_OATC14_PLCA_ID) |
> 1041     			      (plca_cfg->node_id);
> drivers/net/phy/phy-c45.c:1076 in genphy_c45_plca_set_cfg()
> 1070     				return ret;
> 1071
> 1072     			val = ret;
> 1073     		}
> 1074
> 1075     		if (plca_cfg->burst_cnt >= 0)
> vvv     CID 1530573:    (UNINIT)
> vvv     Using uninitialized value "val".
> 1076     			val = (val & ~MDIO_OATC14_PLCA_MAXBC) |
> 1077     			      (plca_cfg->burst_cnt << 8);
> 1078
> 1079     		if (plca_cfg->burst_tmr >= 0)
> 1080     			val = (val & ~MDIO_OATC14_PLCA_BTMR) |
> 1081     			      (plca_cfg->burst_tmr);
> 
> This is not actually creating a real problem because the path leading to
> 'val' being used uninitialized will eventually override the full content
> of that variable before actually using it for writing the register.
> However, the fix is simple and comes at basically no cost.
> 

Makes sense, and its better to be clear, and prevent the introduction of
a bug later if somehow it refactored such that the value is not
re-initialized before use in that case.

Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>

> Signed-off-by: Piergiorgio Beruto <piergiorgio.beruto@...il.com>
> Reported-by: coverity-bot <keescook+coverity-bot@...omium.org>
> Addresses-Coverity-ID: 1530573 ("UNINIT")
> Fixes: 493323416fed ("drivers/net/phy: add helpers to get/set PLCA configuration")
> ---
>  drivers/net/phy/phy-c45.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
> index cff83220595c..9f9565a4819d 100644
> --- a/drivers/net/phy/phy-c45.c
> +++ b/drivers/net/phy/phy-c45.c
> @@ -999,8 +999,8 @@ EXPORT_SYMBOL_GPL(genphy_c45_plca_get_cfg);
>  int genphy_c45_plca_set_cfg(struct phy_device *phydev,
>  			    const struct phy_plca_cfg *plca_cfg)
>  {
> +	u16 val = 0;
>  	int ret;
> -	u16 val;
>  
>  	// PLCA IDVER is read-only
>  	if (plca_cfg->version >= 0)