netdev - Re: [PATCH 1/4] net: fix uninitialised access in mii_nway

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]

Message-ID: <c8ebd8a1-cfdd-4a27-8cb6-114ea60ba294@lunn.ch>
Date: Fri, 11 Apr 2025 03:12:06 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Qasim Ijaz <qasdev00@...il.com>
Cc: Jakub Kicinski <kuba@...nel.org>, andrew+netdev@...n.ch,
	davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com,
	horms@...nel.org, linux-usb@...r.kernel.org, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	syzbot <syzbot+3361c2d6f78a3e0892f9@...kaller.appspotmail.com>
Subject: Re: [PATCH 1/4] net: fix uninitialised access in mii_nway_restart()

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