Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <22e207da-1689-520f-e960-133cb9fc2097@intel.com>
Date:   Tue, 17 Jan 2023 15:14:02 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH net-next 1/1] drivers/phylib: fix coverity issue
Content-Language: en-US
To:     Piergiorgio Beruto <piergiorgio.beruto@gmail.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        "David S. Miller" <davem@davemloft.net>,
        "Eric Dumazet" <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>
CC:     <linux-kernel@vger.kernel.org>, <netdev@vger.kernel.org>,
        Oleksij Rempel <o.rempel@pengutronix.de>,
        <mailhol.vincent@wanadoo.fr>, <sudheer.mogilappagari@intel.com>,
        <sbhatta@marvell.com>, <linux-doc@vger.kernel.org>,
        <wangjie125@huawei.com>, <corbet@lwn.net>, <lkp@intel.com>,
        <gal@nvidia.com>, <gustavoars@kernel.org>, <bagasdotme@gmail.com>
References: <5061b6d09d0cd69c832c9c0f2f1a6848d3a5ab1c.1673991998.git.piergiorgio.beruto@gmail.com>
From:   Jacob Keller <jacob.e.keller@intel.com>
In-Reply-To: <5061b6d09d0cd69c832c9c0f2f1a6848d3a5ab1c.1673991998.git.piergiorgio.beruto@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Vis3SXJhaUNTYnAwR2MvMWNicnBBTDdJOHNBZ3AyMk5CRkhybVdrWmZXUVFk?=
 =?utf-8?B?bEJMQ2dIZ055OFR1THN0Qmp5ZmFXeU5lczhaY1FlMHVUOWRNQ0JkN1E5UmFm?=
 =?utf-8?B?dTB5bEJxdjZMQWZyenhqbGVNVDZUSUJ3Y1VjQ3d4MXBHUG10dWVpemV1bWQx?=
 =?utf-8?B?Z1IxNmJGbFVhYWtVKzV2Vkw4OWE4U2tBb040TzhIQ3UvNVVTcElLOUlyVTAz?=
 =?utf-8?B?L1BjUTAva1l1RjM2M1dDVko3UlBzNjRJUHg0MDljRWVQTzdHYkg5QUhkWXRT?=
 =?utf-8?B?SjJZbUkzZUlmYXZSdy9WT29CQzgycEhjMXVvL0NxajJ4UFdkZ29mdUExMUNk?=
 =?utf-8?B?UWwxZXJWRXptTTFvQmJlUU9CNUlVTFVUR1YrUE9TMThsZVZ0Tm9RSGxsK3V4?=
 =?utf-8?B?czVHQitId3AvWWlDdHJsN25pbGQwdGk1Q0N3NllqS216L0hDamtHSlNHdVhk?=
 =?utf-8?B?Ry9RYWErdkFjS0JJb2dkbkVSN2hUM1FlKzdjaXBZRWM3a295ajZRRkdBNDdU?=
 =?utf-8?B?NkRHL3NUUVdpTFVkUkJaZTRuYWxyQm9ZSlFJTzAzakNoUnR3SWxLanBDdnAv?=
 =?utf-8?B?b1YyK2VyS25yNThsdFdGeUN3dzFaMnU1MlloNHZxVDhrTkpuWDloNWZTOVNE?=
 =?utf-8?B?N1pGYjVnVmhxTlB5aEpFTEE0a0Y1NVp5cjNXZENUMkE4Y05IVDIvNjF5U0U3?=
 =?utf-8?B?ckNSYTJxdkVjNm95Tm1DMjhEZWFBTHJnbVk2QURvVCt6VU9CZlE1Wm1wWlp1?=
 =?utf-8?B?a2o1di9SYUljcHgwNWlWNi96UDFpWlJ4OGFNL1d2NWNqYU40UEdBVjgvUllj?=
 =?utf-8?B?U3VMY2xwbU4vQkpJNVU3Q0xoQWhFM0hrYXV5eGNPQkZ6cTlMRFJ4NERGaVNr?=
 =?utf-8?B?LzdTRmlKSEdOdzlva2ljcUxjYWRtK2JJSjJpTllja2dNME1SbWhTUlNtZm5h?=
 =?utf-8?B?UEhFSldLTnJGNjV1aVVaeGNtditKRHYwaG9HVUZRRUtUZ2dKNGMvbHhtMzhj?=
 =?utf-8?B?MWkwMnJkZ2Z6UEl5V1hXYlEydDRzQnh4SUUvRnBTTngrOVVTUSt0OVlmeVFV?=
 =?utf-8?B?NzJkb2FCdmtrS1YzbG83azBHVm5oYXpIOVVDeHpMZ0xlZkpBdnltekZobmRJ?=
 =?utf-8?B?YTYrZndXT0FyMjVFbWtSYyttTWFQMndrNkVXWkVKNUlMdUZzS3FDL1BvdDF4?=
 =?utf-8?B?Z3I4K1JhaWpnQ20wNFJpdHNCcG9YYS8xb20remNhVmNVRWxoNnpBdFFrMVh0?=
 =?utf-8?B?MzdFcktpS0ZuRmFLbXZoZzFsazZGWTFMK1dPNGkxUmloV3JpbjFPeGxUVXNT?=
 =?utf-8?B?RDNMdEpydWF0YlBFWlBnZHYrZkEweFhha3ZmUHNzTWhiK0RhT3lqdGJkM2ZF?=
 =?utf-8?B?YjRJVnJHa2JqNTk2RGZMQzVUNWxMZjJha0QrYnExajBJdXZiRUtPVFgwdzBK?=
 =?utf-8?B?aC9TbzZrQkoxZ0dlNzZkbVcxeGh2UVRrMVNvWHRrRHdnSDN2UTVEQkZrL3Nn?=
 =?utf-8?B?ZFNFeTh4Ky81NEhFVitkY1RZNjJnZk5xU2xJZDhOZ3dNS3VvQ3p3N2llM2Za?=
 =?utf-8?B?MFIxL2RjVDIzcXRLSUdWaVZ6RDQvN0pleTliWkRQZGJOVDRKdG9UaGd6QWVG?=
 =?utf-8?B?NHRTZEU1bWptRm1TS3V6TW5aWGtKdTUrZFJFU3Rpd0liejNueUdZSkZsQzhL?=
 =?utf-8?B?N2t0S2lwYm5JMlA3M1NXNWVnM0NRSHlCdG9UdXFqdGY5bC9TdXlUSG5qSEJN?=
 =?utf-8?B?a2s1YmRDSXI0eDc2SVZ0K3N5SVBkOCtaNGxMU1VyanVGbllYN3ltM1hLbXVW?=
 =?utf-8?B?THVwRnRGQ3l2aFlaVFBzeWt3a0lsWTl3Z3pyb01xUDVwQ2RleWxsV01kS3dO?=
 =?utf-8?B?dE1EYkVpdjhtdUFFV2VxSk9qNHpYejQ3Q3hrbkRYU2JqOHovazFkTCtuc0J3?=
 =?utf-8?B?dTY2Rld1SUZCMHRCS0krMkVTVEdkc0lwSjJMU1VlU0s2TTJPM3I1U05BNGRZ?=
 =?utf-8?B?b2Rtd2ZLNkcyWk5ieHlmRXZuVjVQWlA4c1JLc2NMczVmRUdBQlY5Z2JBdDZS?=
 =?utf-8?B?bkNWd2Z2M0VJQXdhUFJZQXdsYU9PSUYxRlFBSHJOWStod0ljbVVPN09Fc3Ns?=
 =?utf-8?B?V3g2WlpSNnM5SjIreExQMDRpVGViUE4vK2V6Y25zdWswcHNUWVh6MVBEVXZt?=
 =?utf-8?B?b0E9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: eb4d87ba-42b6-4aaa-b684-08daf8e0874b
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB5089.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Jan 2023 23:14:05.4938
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: R+2Klk0U8TYERjWt/2OA4oSRvMx/B8/RhyRwc6uX3K0XjTZrLYfBO0rpb7Ym8UPkvA+zxRiwbfvqad2bqllJZrEcK4pL08zAZopTvoO5ELg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB4969
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


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@intel.com>

> Signed-off-by: Piergiorgio Beruto <piergiorgio.beruto@gmail.com>
> Reported-by: coverity-bot <keescook+coverity-bot@chromium.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)