Netdev List
 help / color / mirror / Atom feed
From: Carlo Szelinsky <github@szelinsky.de>
To: Oleksij Rempel <o.rempel@pengutronix.de>,
	Kory Maincent <kory.maincent@bootlin.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Cc: Corey Leavitt <corey@leavitt.info>,
	Jonas Jelonek <jelonek.jonas@gmail.com>,
	Simon Horman <horms@kernel.org>,
	Aleksander Jan Bajkowski <olek2@wp.pl>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Carlo Szelinsky <github@szelinsky.de>
Subject: [PATCH net-next v5 5/5] net: phy: release phydev->psec from phy_device_remove() again
Date: Thu, 27 Aug 2026 00:03:44 +0200	[thread overview]
Message-ID: <20260826220344.121865-6-github@szelinsky.de> (raw)
In-Reply-To: <20260826220344.121865-1-github@szelinsky.de>

"net: phy: own phydev->psec via PSE notifier and remove fwnode_mdio
hook" deferred the final pse_control_put() of phydev->psec from
phy_device_remove() to phy_device_release(), so it would run only after
the PSE_UNREGISTERED notifier walk had dropped its bus-iterator reference
on the phy. But bus_for_each_dev() only reaches phys still on the
mdio_bus_type klist: a phy that has been device_del()'d yet is still
pinned (e.g. by an attached netdev) is invisible to the walk, so
phy_pse_detach_one() never clears its phydev->psec. Its deferred put then
runs after pse_controller_unregister() -> pse_release_pis() has freed
pcdev->pi[], and __pse_control_release() dereferences the freed array:
use-after-free.

Put phydev->psec back in phy_device_remove(), before device_del(), so
the detach is synchronous and ordered ahead of the phy leaving the bus;
it can no longer outlive the PSE controller. "net: phy: use a dedicated
mutex instead of rtnl for PSE control attach" replaced rtnl with
phy_pse_lock for the attach/detach, so this put can take that same lock
without the rtnl recursion that originally motivated the deferral, and it
serialises against the notifier walk: whichever runs first clears
phydev->psec, the other sees NULL.

Suggested-by: Paolo Abeni <pabeni@redhat.com>
Link: https://lore.kernel.org/netdev/20260703071025.100797-1-pabeni@redhat.com/
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
 drivers/net/phy/phy_device.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index fa6c3d638b30..dca1b45bbbd2 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -223,19 +223,8 @@ static void phy_mdio_device_free(struct mdio_device *mdiodev)
 
 static void phy_device_release(struct device *dev)
 {
-	struct phy_device *phydev = to_phy_device(dev);
-
-	/* bus_for_each_dev() holds get_device() across each iteration
-	 * step, deferring this release callback until any in-flight PSE
-	 * notifier walk has advanced past this phy. pse_control_put()
-	 * takes pse_list_mutex, so this path must run in sleepable
-	 * context.
-	 */
-	might_sleep();
-	pse_control_put(phydev->psec);
-
 	fwnode_handle_put(dev->fwnode);
-	kfree(phydev);
+	kfree(to_phy_device(dev));
 }
 
 static void phy_mdio_device_remove(struct mdio_device *mdiodev)
@@ -1326,6 +1315,16 @@ EXPORT_SYMBOL(phy_device_register);
 void phy_device_remove(struct phy_device *phydev)
 {
 	unregister_mii_timestamper(phydev->mii_ts);
+
+	/* Detach synchronously, before the phy leaves the bus, so the put cannot
+	 * outlive the PSE controller (an off-bus but still-pinned phy is missed by
+	 * the PSE_UNREGISTERED walk). phy_pse_lock serialises against that walk.
+	 */
+	mutex_lock(&phy_pse_lock);
+	pse_control_put(phydev->psec);
+	phydev->psec = NULL;
+	mutex_unlock(&phy_pse_lock);
+
 	device_del(&phydev->mdio.dev);
 
 	/* Assert the reset signal */
-- 
2.43.0


  parent reply	other threads:[~2026-08-26 22:04 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-26 22:03 [PATCH net-next v5 0/5] net: pse-pd: decouple controller lookup from MDIO probe Carlo Szelinsky
2026-08-26 22:03 ` [PATCH net-next v5 1/5] net: pse-pd: add notifier chain for controller lifecycle events Carlo Szelinsky
2026-08-26 22:03 ` [PATCH net-next v5 2/5] net: pse-pd: fire lifecycle events on controller register/unregister Carlo Szelinsky
2026-08-26 22:03 ` [PATCH net-next v5 3/5] net: phy: own phydev->psec via PSE notifier and remove fwnode_mdio hook Carlo Szelinsky
2026-08-26 22:03 ` [PATCH net-next v5 4/5] net: phy: use a dedicated mutex instead of rtnl for PSE control attach Carlo Szelinsky
2026-08-26 22:03 ` Carlo Szelinsky [this message]
2026-08-27  8:22 ` [PATCH net-next v5 0/5] net: pse-pd: decouple controller lookup from MDIO probe Paolo Abeni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260826220344.121865-6-github@szelinsky.de \
    --to=github@szelinsky.de \
    --cc=andrew+netdev@lunn.ch \
    --cc=corey@leavitt.info \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=horms@kernel.org \
    --cc=jelonek.jonas@gmail.com \
    --cc=kory.maincent@bootlin.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=olek2@wp.pl \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox