Re: [PATCH 1/4] net: fix uninitialised access in mii_nway_restart()

[Andrew Lunn <andrew@lunn.ch>]

On Thu, Apr 10, 2025 at 11:15:23PM +0100, Qasim Ijaz wrote:
> On Tue, Mar 25, 2025 at 06:33:07AM -0700, Jakub Kicinski wrote:
> > On Wed, 19 Mar 2025 11:21:53 +0000 Qasim Ijaz wrote:
> > > --- a/drivers/net/mii.c
> > > +++ b/drivers/net/mii.c
> > > @@ -464,6 +464,8 @@ int mii_nway_restart (struct mii_if_info *mii)
> > >  
> > >  	/* if autoneg is off, it's an error */
> > >  	bmcr = mii->mdio_read(mii->dev, mii->phy_id, MII_BMCR);
> > > +	if (bmcr < 0)
> > > +		return bmcr;
> > >  
> > >  	if (bmcr & BMCR_ANENABLE) {
> > >  		bmcr |= BMCR_ANRESTART;
> > 
> > We error check just one mdio_read() but there's a whole bunch of them
> > in this file. What's the expected behavior then? Are all of them buggy?
> >
>  
> Hi Jakub
>     
> Apologies for my delayed response, I had another look at this and I
> think my patch may be off a bit. You are correct that there are multiple
> mdio_read() calls and looking at the mii.c file we can see that calls to
> functions like mdio_read (and a lot of others) dont check return values.
>   
> So in light of this I think a better patch would be to not edit the 
> mii.c file at all and just make ch9200_mdio_read return 0 on     
> error.

Do you actually have one of these devices? If you do have, an even
better change would be to throwaway the mii code and swap to phylib
and an MDIO bus. You can probably follow smsc95xx.c.

	Andrew