[PATCH net 0/2] net: fec: fix PTP clock teardown races on device removal

[PATCH net 0/2] net: fec: fix PTP clock teardown races on device removal

[Shengzhuo Wei]

fec_drv_remove() calls fec_ptp_stop() before the netdev is unregistered
and freed, but PTP teardown leaves dangling references behind.  The PHC
pointer remains set after ptp_clock_unregister(), so an ethtool -T on the
still-registered netdev can pass it to ptp_clock_index() after the clock
structure has been freed.

An in-flight PPS handler can race ptp_clock_unregister().  The devm-managed
handler also remains registered past free_netdev() and can dereference the
freed netdev before device-managed resources are released.

This series closes both holes at their respective introduction points.
Look up the PHC index by the parent device and clear fep->ptp_clock after
unregistering the PHC, then explicitly free the dedicated PPS interrupt
before the PHC and netdev teardown.

Found by source inspection while reviewing PTP teardown paths.  Verified by
compiling the driver with W=1.  No hardware was available to reproduce the
races.

---
Shengzhuo Wei (2):
      net: fec: don't leave a stale PTP clock pointer after unregister
      net: fec: free the PPS interrupt before tearing down the PHC and netdev

 drivers/net/ethernet/freescale/fec.h      |  1 +
 drivers/net/ethernet/freescale/fec_main.c |  3 +--
 drivers/net/ethernet/freescale/fec_ptp.c  | 15 +++++++++++++--
 3 files changed, 15 insertions(+), 4 deletions(-)
---
base-commit: 548e7bcd0c5460ddcbca9600cea603ebeebf4da7
change-id: 20260901-fec-ptp-pps-event-uaf-dcc71b5e1db0

Best regards,
--  
Shengzhuo Wei <me@cherr.cc>

[PATCH net 1/2] net: fec: don't leave a stale PTP clock pointer after unregister

[Shengzhuo Wei]

fec_drv_remove() calls fec_ptp_stop() before unregister_netdev(), and
fec_ptp_stop() leaves fep->ptp_clock set after ptp_clock_unregister().
An ethtool -T issued while the netdev is still registered then reaches
fec_enet_get_ts_info(), which passes the stale pointer to
ptp_clock_index() after the clock structure has been freed.

Query the PHC index through ptp_clock_index_by_dev() instead.  The lookup
holds a reference to the matching PTP class device while reading its index,
so concurrent unregister cannot free it underneath the lookup.  It returns
-1 once no clock is registered.  Clear fep->ptp_clock after unregistering
it as well, so the driver state no longer retains the invalid pointer.

Fixes: 32cba57ba74b ("net: fec: introduce fec_ptp_stop and use in probe fail path")
Cc: stable@vger.kernel.org
Assisted-by: GLM:5.3
Signed-off-by: Shengzhuo Wei <me@cherr.cc>
---
 drivers/net/ethernet/freescale/fec_main.c | 3 +--
 drivers/net/ethernet/freescale/fec_ptp.c  | 4 +++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 794ec427b0ee..0606559d495c 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3337,8 +3337,7 @@ static int fec_enet_get_ts_info(struct net_device *ndev,
 					SOF_TIMESTAMPING_TX_HARDWARE |
 					SOF_TIMESTAMPING_RX_HARDWARE |
 					SOF_TIMESTAMPING_RAW_HARDWARE;
-		if (fep->ptp_clock)
-			info->phc_index = ptp_clock_index(fep->ptp_clock);
+		info->phc_index = ptp_clock_index_by_dev(&fep->pdev->dev);
 
 		info->tx_types = (1 << HWTSTAMP_TX_OFF) |
 				 (1 << HWTSTAMP_TX_ON);
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index 56801c2009d5..0036549974fd 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -861,6 +861,8 @@ void fec_ptp_stop(struct platform_device *pdev)
 
 	cancel_delayed_work_sync(&fep->time_keep);
 	hrtimer_cancel(&fep->perout_timer);
-	if (fep->ptp_clock)
+	if (fep->ptp_clock) {
 		ptp_clock_unregister(fep->ptp_clock);
+		fep->ptp_clock = NULL;
+	}
 }

-- 
2.47.3

[PATCH net 2/2] net: fec: free the PPS interrupt before tearing down the PHC and netdev

[Shengzhuo Wei]

The dedicated PPS interrupt is devm-managed, so its handler remains
registered until device-managed resources are released after the remove
callback returns.  It therefore outlives both fec_ptp_stop() and
free_netdev().

A handler already in flight can observe pps_enable before fec_ptp_stop()
clears it and call ptp_clock_event() concurrently with
ptp_clock_unregister().  A handler invoked after free_netdev() but before
device-managed cleanup dereferences the freed netdev.

Record the IRQ after a successful request and release it explicitly in
fec_ptp_stop(), before unregistering the PHC.  devm_free_irq() removes the
handler and waits for any running instance, so none can still execute when
PHC teardown begins.  The IRQ is requested without IRQF_SHARED, so its
release does not affect another handler.

Fixes: 4ad1ceec05e4 ("net: fec: Let fec_ptp have its own interrupt routine")
Cc: stable@vger.kernel.org
Assisted-by: GLM:5.3
Signed-off-by: Shengzhuo Wei <me@cherr.cc>
---
 drivers/net/ethernet/freescale/fec.h     |  1 +
 drivers/net/ethernet/freescale/fec_ptp.c | 11 ++++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 7176803146f3..960b9f01c531 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -670,6 +670,7 @@ struct fec_enet_private {
 
 	/* pps  */
 	int pps_channel;
+	int pps_irq;
 	unsigned int reload_period;
 	int pps_enable;
 	unsigned int next_counter;
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index 0036549974fd..5b1d58c85fcd 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -787,9 +787,12 @@ void fec_ptp_init(struct platform_device *pdev, int irq_idx)
 	if (irq >= 0) {
 		ret = devm_request_irq(&pdev->dev, irq, fec_pps_interrupt,
 				       0, pdev->name, ndev);
-		if (ret < 0)
+		if (ret < 0) {
 			dev_warn(&pdev->dev, "request for pps irq failed(%d)\n",
 				 ret);
+		} else {
+			fep->pps_irq = irq;
+		}
 	}
 
 	fep->ptp_clock = ptp_clock_register(&fep->ptp_caps, &pdev->dev);
@@ -861,6 +864,12 @@ void fec_ptp_stop(struct platform_device *pdev)
 
 	cancel_delayed_work_sync(&fep->time_keep);
 	hrtimer_cancel(&fep->perout_timer);
+
+	if (fep->pps_irq > 0) {
+		devm_free_irq(&pdev->dev, fep->pps_irq, ndev);
+		fep->pps_irq = 0;
+	}
+
 	if (fep->ptp_clock) {
 		ptp_clock_unregister(fep->ptp_clock);
 		fep->ptp_clock = NULL;

-- 
2.47.3

RE: [PATCH net 1/2] net: fec: don't leave a stale PTP clock pointer after unregister

[Wei Fang]

> fec_drv_remove() calls fec_ptp_stop() before unregister_netdev(), and
> fec_ptp_stop() leaves fep->ptp_clock set after ptp_clock_unregister().
> An ethtool -T issued while the netdev is still registered then reaches
> fec_enet_get_ts_info(), which passes the stale pointer to
> ptp_clock_index() after the clock structure has been freed.
> 
> Query the PHC index through ptp_clock_index_by_dev() instead.  The lookup
> holds a reference to the matching PTP class device while reading its index,
> so concurrent unregister cannot free it underneath the lookup.  It returns
> -1 once no clock is registered.  Clear fep->ptp_clock after unregistering
> it as well, so the driver state no longer retains the invalid pointer.

Why not move fec_ptp_stop() after unregister_netdev()?

Re: [PATCH net 1/2] net: fec: don't leave a stale PTP clock pointer after unregister

[Shengzhuo Wei]

On 2026-09-04 03:15, Wei Fang wrote:
> Why not move fec_ptp_stop() after unregister_netdev()?

Good point. I was focused on making the PHC index lookup safe
independently of the teardown ordering and followed the approach used by
similar fixes, so I overlooked the simpler solution here.

Moving fec_ptp_stop() after unregister_netdev() is cleaner and restores
the proper lifetime ordering. I will rework patch 1 accordingly and drop
the ptp_clock_index_by_dev() change in v2.

Thanks,
Shengzhuo