[PATCH] net: dsa: vsc73xxx: Fix uninitalized 'val' in vsc73xx_adjust_link

Using uninitialized variable after calls vsc73xx_read 
without error checking may cause incorrect driver behavior.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 95711cd5f0b4 ("net: dsa: vsc73xx: Split vsc73xx driver")
Signed-off-by: Danila Chernetsov <redacted>
---
 drivers/net/dsa/vitesse-vsc73xx-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
index ae55167ce0a6..729005d6cb7e 100644
--- a/drivers/net/dsa/vitesse-vsc73xx-core.c
+++ b/drivers/net/dsa/vitesse-vsc73xx-core.c

@@ -758,7 +758,7 @@ static void vsc73xx_adjust_link(struct dsa_switch *ds, int port,
 				struct phy_device *phydev)
 {
 	struct vsc73xx *vsc = ds->priv;
-	u32 val;
+	u32 val = 0;
 
 	/* Special handling of the CPU-facing port */
 	if (port == CPU_PORT) {

-- 
2.25.1

On Sun, Mar 12, 2023 at 03:50:08PM +0000, Danila Chernetsov wrote:

Using uninitialized variable after calls vsc73xx_read 
without error checking may cause incorrect driver behavior.

I wonder if it is:
a) intentional that these calls are not checked for errors
b) errors can occur in these call paths

On Mon, 13 Mar 2023 19:59:27 +0100 Simon Horman wrote:

On Sun, Mar 12, 2023 at 03:50:08PM +0000, Danila Chernetsov wrote:

quoted

Using uninitialized variable after calls vsc73xx_read 
without error checking may cause incorrect driver behavior.  

I wonder if it is:
a) intentional that these calls are not checked for errors
b) errors can occur in these call paths

At the very least we should handle the error rather than silencing 
the static checker complain by picking a semi-random init value for 
the entire function.

On Mon, Mar 13, 2023 at 07:59:27PM +0100, Simon Horman wrote:

On Sun, Mar 12, 2023 at 03:50:08PM +0000, Danila Chernetsov wrote:

quoted

Using uninitialized variable after calls vsc73xx_read 
without error checking may cause incorrect driver behavior.

I wonder if it is:
a) intentional that these calls are not checked for errors

probably no; this is precisely the only vsc73xx_read() call whose return
code is ignores. I'd say it partly has to do with the fact that vsc73xx_adjust_link()
returns void, so the author was thinking there'd be no point in checking
for errors, but there clearly is

b) errors can occur in these call paths

probably yes; one of the instantiations of vsc73xx is over SPI, where
the controller can time out, etc.