netdev - RE: [PATCH net] phy: fix null pointer issue in phy_attach

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] [day] [month] [year] [list]

Message-ID: <IA1PR11MB62895A8DCF184E77462FC59989E82@IA1PR11MB6289.namprd11.prod.outlook.com>
Date: Fri, 31 Jan 2025 14:57:21 +0000
From: "Joshi, Sreedevi" <sreedevi.joshi@...el.com>
To: Andrew Lunn <andrew@...n.ch>
CC: sreedevi.joshi <joshisre@...mtp.an.intel.com>, "hkallweit1@...il.com"
	<hkallweit1@...il.com>, "linux@...linux.org.uk" <linux@...linux.org.uk>,
	"edumazet@...gle.com" <edumazet@...gle.com>, "kuba@...nel.org"
	<kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH net] phy: fix null pointer issue in phy_attach_direct()


> -----Original Message-----
> From: Andrew Lunn <andrew@...n.ch>
> Sent: Thursday, January 30, 2025 2:52 PM
> To: Joshi, Sreedevi <sreedevi.joshi@...el.com>
> Cc: sreedevi.joshi <joshisre@...mtp.an.intel.com>; hkallweit1@...il.com; linux@...linux.org.uk; edumazet@...gle.com;
> kuba@...nel.org; pabeni@...hat.com; netdev@...r.kernel.org
> Subject: Re: [PATCH net] phy: fix null pointer issue in phy_attach_direct()
> 
> On Thu, Jan 30, 2025 at 07:10:49PM +0000, Joshi, Sreedevi wrote:
> >
> > > -----Original Message-----
> > > From: Andrew Lunn <andrew@...n.ch>
> > > Sent: Wednesday, January 29, 2025 2:14 PM
> > > To: sreedevi.joshi <joshisre@...mtp.an.intel.com>
> > > Cc: hkallweit1@...il.com; linux@...linux.org.uk; edumazet@...gle.com; kuba@...nel.org; pabeni@...hat.com;
> > > netdev@...r.kernel.org; Joshi, Sreedevi <sreedevi.joshi@...el.com>
> > > Subject: Re: [PATCH net] phy: fix null pointer issue in phy_attach_direct()
> > >
> > > On Wed, Jan 29, 2025 at 12:36:38PM -0600, sreedevi.joshi wrote:
> > > > From: Sreedevi Joshi <sreedevi.joshi@...el.com>
> > > >
> > > > When attaching a fixed phy to devices like veth
> > >
> > > Humm. Zoom out. What is the big picture? Why would a veth need a PHY?
> > >
> > > 	Andrew
> > []
> > This issue was encountered when working on a POC to demo the mii_timestamper timestamp
> > callback hooks mechanism. We are using veth pairs as we don't have the HW yet. In this demo,
> > we connect a fixed PHY to veth and attach mii_timestamper hooks that way. However, as veth device
> > (like any other virtual interfaces) does not have a parent, it causes Kernel Oops and on our system
> > it needs a reboot to recover the system. With this check in place,
> > we could connect fixed PHY and mii_timestamper hooks successfully. I understand
> > it is not a common practice to attach a PHY to a virtual interface. However, having a check for NULL
> > before accessing the member will be good to avoid issues.
> 
> Well, there is a flip side to this. You are doing something which does
> not make sense. Getting an Opps is a good indication you are doing
> something you should not. And the Opps makes it easy to
> debug. Silently ignoring the problem makes it a lot harder to find.
> 
Ok. makes sense.

> Is there a legitimate use case for a physical network device without a
> parent device? It looks like phy_attach_direct() has been referencing
> the parent since December 2016, so given its history i'm not sure
> there is a legitimate use case.
> 
> I assume when you get real hardware you will have both a parent and a
> PHY?
> 
> 	Andrew

Yes. This solution is a POC and with real HW we will attach hooks on an interface
that has real PHY.

Thank you for taking time to review the patch and for providing feedback!

Sreedevi