diff mbox series

[net] phy: fix null pointer issue in phy_attach_direct()

Message ID 20250129183638.695010-1-sreedevi.joshi@intel.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net] phy: fix null pointer issue in phy_attach_direct() | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers fail 3 blamed authors not CCed: f.fainelli@gmail.com ioana.ciornei@nxp.com olteanv@gmail.com; 3 maintainers not CCed: f.fainelli@gmail.com ioana.ciornei@nxp.com olteanv@gmail.com
netdev/build_clang success Errors and warnings before: 2 this patch: 2
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 8 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 39 this patch: 39
netdev/source_inline success Was 0 now: 0

Commit Message

sreedevi.joshi Jan. 29, 2025, 6:36 p.m. UTC 
From: Sreedevi Joshi <sreedevi.joshi@intel.com>

When attaching a fixed phy to devices like veth, it is
possible that there is no parent. The logic in
phy_attach_direct() tries to access the driver member
without checking for the null. This causes segfault in the
case of fixed phy.

Checks are in place now to ensure the parent is not null
before accessing to address this scenario.

Fixes: 2db2d9d1ac37 ("net: phy: Guard against the presence of a netdev")
Signed-off-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
---
 drivers/net/phy/phy_device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andrew Lunn Jan. 29, 2025, 7:14 p.m. UTC | #1 
On Wed, Jan 29, 2025 at 12:36:38PM -0600, sreedevi.joshi wrote:
> From: Sreedevi Joshi <sreedevi.joshi@intel.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
Russell King (Oracle) Jan. 29, 2025, 7:29 p.m. UTC | #2 
On Wed, Jan 29, 2025 at 12:36:38PM -0600, sreedevi.joshi wrote:
> From: Sreedevi Joshi <sreedevi.joshi@intel.com>
> 
> When attaching a fixed phy to devices like veth, it is
> possible that there is no parent. The logic in
> phy_attach_direct() tries to access the driver member
> without checking for the null. This causes segfault in the
> case of fixed phy.

Kernel mode doesn't segfault. That's a userspace thing. Kernel mode
oopses.

I'm confused. You mention veth, which presumably is drivers/net/veth.c.
Grepping this driver for "phy" returns nothing. So how can veth be
broken by a phylib change?
diff mbox series

Patch

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 46713d27412b..be813962cb89 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1471,7 +1471,7 @@  int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	 * our own module->refcnt here, otherwise we would not be able to
 	 * unload later on.
 	 */
-	if (dev)
+	if (dev && dev->dev.parent)
 		ndev_owner = dev->dev.parent->driver->owner;
 	if (ndev_owner != bus->owner && !try_module_get(bus->owner)) {
 		phydev_err(phydev, "failed to get the bus module\n");