[PATCH v2 net] ptp: netc: fix potential interrupt storm caused by incorrect unbind order

[PATCH v2 net] ptp: netc: fix potential interrupt storm caused by incorrect unbind order

[wei.fang]

From: Wei Fang <wei.fang@nxp.com>

In netc_timer_remove(), hardware interrupts are disabled by clearing
TMR_TEMASK before ptp_clock_unregister() is called. This may cause a
race condition during driver unbind that could leave hardware interrupts
active. For example, a concurrent PTP_CLK_REQ_EXTTS ioctl can re-enable
TMR_TEMASK after it has been cleared, leaving a pending hardware
interrupt when the driver unbinds.

Since the NETC Timer does not support PCIe FLR, hardware state is not
reset during probe. When the driver is rebound and the IRQ is registered,
the pending interrupt fires immediately. At that point priv->tmr_emask
is still zero, so netc_timer_isr() does not clear the interrupt status
and unconditionally returns IRQ_HANDLED, resulting in an uninterruptible
infinite interrupt storm.

Fix this in several ways. First, request the IRQ with IRQF_NO_AUTOEN so
it is not enabled when request_irq() runs, and clear TMR_TEMASK in
netc_timer_init() before enabling it. The IRQ is only enabled at the end
of probe once the timer has been reprogrammed and the PTP clock has been
registered. This ensures a stale pending interrupt from a previous unbind
or an unclean shutdown cannot be delivered before the driver is fully
initialized.

Second, in netc_timer_remove() call disable_irq() before
ptp_clock_unregister() and move the TMR_TEMASK/TMR_CTRL clearing after
it. disable_irq() masks the line and waits for any in-flight
netc_timer_isr() to finish, so no ISR can dereference priv->clock after
ptp_clock_unregister() has freed it. Unregistering the PTP clock before
clearing the mask also guarantees that no in-flight or concurrent ioctl
can re-enable hardware interrupts.

Finally, return IRQ_NONE from netc_timer_isr() when the masked event
status is zero, so the kernel's spurious interrupt detection can disable
a stuck line instead of looping forever.

Fixes: 671e266835b8 ("ptp: netc: add periodic pulse output support")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260720012508.23227-1-wei.fang%40oss.nxp.com
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
v2:
1. netc_timer_remove(): add disable_irq() before ptp_clock_unregister()
   to avoid an ISR touching priv->clock after it is freed.
2. request_irq(): use IRQF_NO_AUTOEN so the IRQ is not enabled too early.
3. netc_timer_free_msix_irq(): drop redundant disable_irq() (free_irq()
   already handles it).
4. netc_timer_probe(): enable_irq() only at the end of probe.
5. netc_timer_init(): clear TMR_TEMASK before enabling the IRQ.
6. netc_timer_isr(): return IRQ_NONE when no masked event is pending.
7. Commit message: rewritten to describe all of the above.
---
 drivers/ptp/ptp_netc.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
index 5e381c354d74..1c20d7efab92 100644
--- a/drivers/ptp/ptp_netc.c
+++ b/drivers/ptp/ptp_netc.c
@@ -769,6 +769,7 @@ static void netc_timer_init(struct netc_timer *priv)
 		   TMR_CTRL_TE | TMR_CTRL_FS;
 	netc_timer_wr(priv, NETC_TMR_CTRL, tmr_ctrl);
 	netc_timer_wr(priv, NETC_TMR_PRSC, priv->oclk_prsc);
+	netc_timer_wr(priv, NETC_TMR_TEMASK, 0);
 
 	/* Disable FIPER by default */
 	fiper_ctrl = netc_timer_rd(priv, NETC_TMR_FIPER_CTRL);
@@ -901,6 +902,11 @@ static irqreturn_t netc_timer_isr(int irq, void *data)
 	/* Clear interrupts status */
 	netc_timer_wr(priv, NETC_TMR_TEVENT, tmr_event);
 
+	if (!tmr_event) {
+		spin_unlock(&priv->lock);
+		return IRQ_NONE;
+	}
+
 	if (tmr_event & TMR_TEVENT_ALMEN(0))
 		netc_timer_alarm_write(priv, NETC_TMR_DEFAULT_ALARM, 0);
 
@@ -936,7 +942,8 @@ static int netc_timer_init_msix_irq(struct netc_timer *priv)
 	}
 
 	priv->irq = pci_irq_vector(pdev, 0);
-	err = request_irq(priv->irq, netc_timer_isr, 0, priv->irq_name, priv);
+	err = request_irq(priv->irq, netc_timer_isr, IRQF_NO_AUTOEN,
+			  priv->irq_name, priv);
 	if (err) {
 		dev_err(&pdev->dev, "request_irq() failed\n");
 		pci_free_irq_vectors(pdev);
@@ -951,7 +958,6 @@ static void netc_timer_free_msix_irq(struct netc_timer *priv)
 {
 	struct pci_dev *pdev = priv->pdev;
 
-	disable_irq(priv->irq);
 	free_irq(priv->irq, priv);
 	pci_free_irq_vectors(pdev);
 }
@@ -1005,6 +1011,8 @@ static int netc_timer_probe(struct pci_dev *pdev,
 		goto free_msix_irq;
 	}
 
+	enable_irq(priv->irq);
+
 	return 0;
 
 free_msix_irq:
@@ -1019,9 +1027,10 @@ static void netc_timer_remove(struct pci_dev *pdev)
 {
 	struct netc_timer *priv = pci_get_drvdata(pdev);
 
+	disable_irq(priv->irq);
+	ptp_clock_unregister(priv->clock);
 	netc_timer_wr(priv, NETC_TMR_TEMASK, 0);
 	netc_timer_wr(priv, NETC_TMR_CTRL, 0);
-	ptp_clock_unregister(priv->clock);
 	netc_timer_free_msix_irq(priv);
 	netc_timer_pci_remove(pdev);
 }
-- 
2.34.1

Re: [PATCH v2 net] ptp: netc: fix potential interrupt storm caused by incorrect unbind order

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver fails to clear the hardware interrupt status register (NETC_TMR_TEVENT) during initialization and only clears unmasked events in the ISR, permanently leaving masked events pending.
--

--- Patch [1]: [PATCH v2 net] ptp: netc: fix potential interrupt storm caused by incorrect unbind order ---
commit 31de617b2b5f4f30ac1afdcfbd8ebfecdf0a6a03
Author: Wei Fang <wei.fang@nxp.com>

ptp: netc: fix potential interrupt storm caused by incorrect unbind order

This commit attempts to fix an interrupt storm during driver unbind by
requesting the IRQ with IRQF_NO_AUTOEN and ensuring hardware interrupts
are disabled before unregistering the PTP clock. It also adds logic to
return IRQ_NONE when no unmasked events are pending.

Closes: https://sashiko.dev/#/patchset/20260720012508.23227-1-wei.fang%40oss.nxp.com

> diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
> --- a/drivers/ptp/ptp_netc.c
> +++ b/drivers/ptp/ptp_netc.c
> @@ -769,6 +769,7 @@ static void netc_timer_init(struct netc_timer *priv)
>  		   TMR_CTRL_TE | TMR_CTRL_FS;
>  	netc_timer_wr(priv, NETC_TMR_CTRL, tmr_ctrl);
>  	netc_timer_wr(priv, NETC_TMR_PRSC, priv->oclk_prsc);
> +	netc_timer_wr(priv, NETC_TMR_TEMASK, 0);

[Severity: High]
This is a pre-existing issue, but does this initialization also need to clear
the event register (NETC_TMR_TEVENT)?

If the hardware holds pending events from a previous unbind or an unclean
shutdown, clearing the mask here won't clear the pending bits themselves,
leaving stale interrupt status in the hardware.

[ ... ]

> @@ -901,6 +902,11 @@ static irqreturn_t netc_timer_isr(int irq, void *data)
>  	/* Clear interrupts status */
>  	netc_timer_wr(priv, NETC_TMR_TEVENT, tmr_event);
>  
> +	if (!tmr_event) {
> +		spin_unlock(&priv->lock);
> +		return IRQ_NONE;
> +	}
> +

[Severity: High]
This is a pre-existing issue, but does writing back the masked tmr_event
fail to clear pending but masked hardware events?

Looking earlier in netc_timer_isr(), tmr_event is masked before being written
back to the register:

    tmr_event = netc_timer_rd(priv, NETC_TMR_TEVENT);
    tmr_event &= priv->tmr_emask;
    /* Clear interrupts status */
    netc_timer_wr(priv, NETC_TMR_TEVENT, tmr_event);

If NETC_TMR_TEVENT is a Write-1-to-Clear register, writing back 0 for
any masked events means they are never cleared. 

If priv->tmr_emask is 0, the ISR will write 0 to NETC_TMR_TEVENT and then
return IRQ_NONE without clearing the actual hardware state. Could this
cause the unacknowledged hardware interrupt to re-trigger indefinitely,
forcing the kernel's spurious interrupt detector to permanently disable the
IRQ line?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727060348.1887464-1-wei.fang@oss.nxp.com?part=1