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

Message-ID: <6f9865c7-830a-4f4d-949a-ea073ead994f@lunn.ch>
Date: Thu, 30 Jan 2025 20:52:22 +0100
From: Andrew Lunn <andrew@...n.ch>
To: "Joshi, Sreedevi" <sreedevi.joshi@...el.com>
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()

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.

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