[PATCH net v2 0/2] net: phy: xilinx-gmii2rgmii: Fix PHY data ownership and removal

[PATCH net v2 0/2] net: phy: xilinx-gmii2rgmii: Fix PHY data ownership and removal

[Vineeth Karumanchi]

The Xilinx GMII-to-RGMII converter copies the attached PHY driver and
replaces its read_status and set_loopback callbacks. This series addresses
two problems in that arrangement: overwriting driver data belonging to
the external PHY, and leaving phydev->drv pointing at freed converter
memory after converter removal.

Patch 1 retrieves the converter private data from its embedded phy_driver
using container_of_const(), preserving the external PHY's MDIO driver-data
field. It fixes the overwrite introduced by commit 168f7a161608 ("net: phy:
gmii2rgmii: Dont use priv field in phy device").

Patch 2 stores private data on the converter's own MDIO device and adds a
remove callback. It restores the original PHY driver under phydev->lock
only if the converter's copy is still installed, and releases the reference
acquired by of_phy_find_device(). This addresses the stale pointer observed
during PHY state-machine polling after converter-only unbind. The missing
removal cleanup dates back to commit f411a6160bd4 ("net: phy: Add
gmiitorgmii converter support"). Apply the patches in order.

Changes in v2:
- Patch 1: unchanged.
- Patch 2: restore the original PHY driver only if phydev->drv still
  points to the converter's copy.
- Patch 2: update the comment and commit message to describe serialization
  against PHY callbacks that hold phydev->lock.

Link to v1: https://lore.kernel.org/netdev/20261001074718.3944521-1-vineeth.karumanchi@amd.com/

Vineeth Karumanchi (2):
  net: phy: xilinx-gmii2rgmii: Avoid overwriting PHY drvdata
  net: phy: xilinx-gmii2rgmii: Restore PHY driver on remove

 drivers/net/phy/xilinx_gmii2rgmii.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)


base-commit: 23609bce9e1de525d1d0e73fc68c6e7971d0b49e
-- 
2.43.0

[PATCH net v2 1/2] net: phy: xilinx-gmii2rgmii: Avoid overwriting PHY drvdata

[Vineeth Karumanchi]

The GMII-to-RGMII converter wraps the attached PHY driver by copying
its phy_driver structure and replacing the read_status and set_loopback
callbacks.

To access the converter private data from these callbacks, the driver
currently stores it in the attached PHY's MDIO driver-data field. This
field belongs to the underlying PHY driver and may already contain its
private data. Overwriting it can therefore cause the PHY driver to
retrieve an unexpected pointer and behave incorrectly.

The converter-specific phy_driver is embedded in struct gmii2rgmii and
installed as phydev->drv. Use container_of_const() to retrieve the
enclosing gmii2rgmii structure from phydev->drv instead of using the
PHY's driver-data field.

Update xgmiitorgmii_configure() to accept a const pointer accordingly.

Fixes: 168f7a161608 ("net: phy: gmii2rgmii: Dont use priv field in phy device")
Signed-off-by: Vineeth Karumanchi <vineeth.karumanchi@amd.com>
---
 drivers/net/phy/xilinx_gmii2rgmii.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c b/drivers/net/phy/xilinx_gmii2rgmii.c
index 2024d8ef36d9..61f71e977a57 100644
--- a/drivers/net/phy/xilinx_gmii2rgmii.c
+++ b/drivers/net/phy/xilinx_gmii2rgmii.c
@@ -28,7 +28,7 @@ struct gmii2rgmii {
 	struct mdio_device *mdio;
 };
 
-static void xgmiitorgmii_configure(struct gmii2rgmii *priv, int speed)
+static void xgmiitorgmii_configure(const struct gmii2rgmii *priv, int speed)
 {
 	struct mii_bus *bus = priv->mdio->bus;
 	int addr = priv->mdio->addr;
@@ -49,7 +49,9 @@ static void xgmiitorgmii_configure(struct gmii2rgmii *priv, int speed)
 
 static int xgmiitorgmii_read_status(struct phy_device *phydev)
 {
-	struct gmii2rgmii *priv = mdiodev_get_drvdata(&phydev->mdio);
+	const struct gmii2rgmii *priv = container_of_const(phydev->drv,
+							   struct gmii2rgmii,
+							   conv_phy_drv);
 	int err;
 
 	if (priv->phy_drv->read_status)
@@ -67,7 +69,9 @@ static int xgmiitorgmii_read_status(struct phy_device *phydev)
 static int xgmiitorgmii_set_loopback(struct phy_device *phydev, bool enable,
 				     int speed)
 {
-	struct gmii2rgmii *priv = mdiodev_get_drvdata(&phydev->mdio);
+	const struct gmii2rgmii *priv = container_of_const(phydev->drv,
+							   struct gmii2rgmii,
+							   conv_phy_drv);
 	int err;
 
 	if (priv->phy_drv->set_loopback)
@@ -123,7 +127,6 @@ static int xgmiitorgmii_probe(struct mdio_device *mdiodev)
 	       sizeof(struct phy_driver));
 	priv->conv_phy_drv.read_status = xgmiitorgmii_read_status;
 	priv->conv_phy_drv.set_loopback = xgmiitorgmii_set_loopback;
-	mdiodev_set_drvdata(&priv->phy_dev->mdio, priv);
 	priv->phy_dev->drv = &priv->conv_phy_drv;
 
 	return 0;
-- 
2.43.0

[PATCH net v2 2/2] net: phy: xilinx-gmii2rgmii: Restore PHY driver on remove

[Vineeth Karumanchi]

The GMII-to-RGMII converter replaces phydev->drv with a modified copy
of the attached PHY driver. This copied driver is embedded in the
converter's private data and is released when the converter is
removed.

Without a remove callback, phydev->drv continues to point to the freed
copy after the converter is unbound. A subsequent PHY operation can
dereference this stale pointer and result in a use-after-free.

With Generic KASAN enabled, unbinding only the converter while the
external PHY remains active produces the following report (abridged):

  BUG: KASAN: slab-use-after-free in phy_check_link_status+0x2d8/0x338
  Read of size 8 at addr ffff000006c359b0 by task kworker/2:0/27
  Workqueue: events_power_efficient phy_state_machine
  Call trace:
   phy_check_link_status+0x2d8/0x338
   _phy_state_machine+0xdc/0xa4c
   phy_state_machine+0x2c/0x70
   process_one_work+0x554/0xe44
   worker_thread+0x6d0/0x1180
   kthread+0x2e8/0x5d4
   ret_from_fork+0x10/0x20

  Allocated by task 55:
   ...
   devm_kmalloc+0xac/0x2ac
   xgmiitorgmii_probe+0xa0/0x37c
   mdio_probe+0x68/0xb4
   ...

  Freed by task 642:
   ...
   kfree+0x14c/0x38c
   release_nodes+0xb4/0x1e0
   devres_release_all+0x140/0x1f4
   device_unbind_cleanup+0x20/0x190
   device_release_driver_internal+0x344/0x460
   device_driver_detach+0x3c/0x54
   unbind_store+0xe0/0xf8
   ...

Store the converter private data in its own MDIO device and add a
remove callback. Restore the attached PHY's original driver only if
phydev->drv still points to the converter's copy. Hold phydev->lock to
serialize the restore against PHY callbacks that take that mutex.

Also release the device reference acquired by of_phy_find_device().

Fixes: f411a6160bd4 ("net: phy: Add gmiitorgmii converter support")
Signed-off-by: Vineeth Karumanchi <vineeth.karumanchi@amd.com>
---
Changes in v2:
- Restore the original PHY driver only if phydev->drv still points to
  the converter's copy.
- Update the comment and commit message to describe serialization against
  PHY callbacks that hold phydev->lock.

Link to v1: https://lore.kernel.org/netdev/20261001074718.3944521-1-vineeth.karumanchi@amd.com/

 drivers/net/phy/xilinx_gmii2rgmii.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c b/drivers/net/phy/xilinx_gmii2rgmii.c
index 61f71e977a57..9dcdb91cd441 100644
--- a/drivers/net/phy/xilinx_gmii2rgmii.c
+++ b/drivers/net/phy/xilinx_gmii2rgmii.c
@@ -128,10 +128,27 @@ static int xgmiitorgmii_probe(struct mdio_device *mdiodev)
 	priv->conv_phy_drv.read_status = xgmiitorgmii_read_status;
 	priv->conv_phy_drv.set_loopback = xgmiitorgmii_set_loopback;
 	priv->phy_dev->drv = &priv->conv_phy_drv;
+	mdiodev_set_drvdata(mdiodev, priv);
 
 	return 0;
 }
 
+static void xgmiitorgmii_remove(struct mdio_device *mdiodev)
+{
+	struct gmii2rgmii *priv = mdiodev_get_drvdata(mdiodev);
+
+	/*
+	 * Restore the original driver only if the converter's copy is still
+	 * installed. Serialize against PHY callbacks that hold phydev->lock.
+	 */
+	mutex_lock(&priv->phy_dev->lock);
+	if (priv->phy_dev->drv == &priv->conv_phy_drv)
+		priv->phy_dev->drv = priv->phy_drv;
+	mutex_unlock(&priv->phy_dev->lock);
+
+	put_device(&priv->phy_dev->mdio.dev);
+}
+
 static const struct of_device_id xgmiitorgmii_of_match[] = {
 	{ .compatible = "xlnx,gmii-to-rgmii-1.0" },
 	{},
@@ -140,6 +157,7 @@ MODULE_DEVICE_TABLE(of, xgmiitorgmii_of_match);
 
 static struct mdio_driver xgmiitorgmii_driver = {
 	.probe	= xgmiitorgmii_probe,
+	.remove	= xgmiitorgmii_remove,
 	.mdiodrv.driver = {
 		.name = "xgmiitorgmii",
 		.of_match_table = xgmiitorgmii_of_match,
-- 
2.43.0