[PATCH net-next v2] net: fec: Propagate PTP initialization errors

[PATCH net-next v2] net: fec: Propagate PTP initialization errors

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Change fec_ptp_init() to return an error code instead of silently
ignoring failures during PTP initialization.

The PPS IRQ is not required for the FEC/PTP functionality, so its
absence should not make the probe fail. However, an unavailable
optional IRQ should be distinguished from an actual error returned
during the IRQ lookup.

If a platform does not support the PPS IRQ, it can omit the IRQ from
its device tree and the optional lookup will return -ENXIO. Propagate
other errors from the IRQ lookup instead of silently ignoring them.

Also propagate failures from devm_request_irq() and ptp_clock_register().

Update the function declaration in fec.h accordingly.

Found by manual code inspection.

Fixes: b86bcb299092 ("net: fec_ptp: Use platform_get_irq_xxx_optional() to avoid error message")
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---

Link v1: 
https://lore.kernel.org/all/20260820111416.97917-1-phucduc.bui@gmail.com/
Changes in v2:
 - Squash the two patches into one.
 - Add error handling for platform_get_irq_byname_optional().
 - Fix error handling for ptp_clock_register().

 drivers/net/ethernet/freescale/fec.h      |  2 +-
 drivers/net/ethernet/freescale/fec_main.c |  7 +++++--
 drivers/net/ethernet/freescale/fec_ptp.c  | 22 ++++++++++++++--------
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 7176803146f3..8831da37326b 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -692,7 +692,7 @@ struct fec_enet_private {
 	u64 ethtool_stats[];
 };
 
-void fec_ptp_init(struct platform_device *pdev, int irq_idx);
+int fec_ptp_init(struct platform_device *pdev, int irq_idx);
 void fec_ptp_restore_state(struct fec_enet_private *fep);
 void fec_ptp_save_state(struct fec_enet_private *fep);
 void fec_ptp_stop(struct platform_device *pdev);
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 794ec427b0ee..c6e29b5c2abb 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -5384,8 +5384,11 @@ fec_probe(struct platform_device *pdev)
 		goto failed_reset;
 
 	irq_cnt = fec_enet_get_irq_cnt(pdev);
-	if (fep->bufdesc_ex)
-		fec_ptp_init(pdev, irq_cnt);
+	if (fep->bufdesc_ex) {
+		ret = fec_ptp_init(pdev, irq_cnt);
+		if (ret)
+			goto failed_reset;
+	}
 
 	ret = fec_enet_init(ndev);
 	if (ret)
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index 56801c2009d5..557a76797eba 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -736,7 +736,7 @@ static irqreturn_t fec_pps_interrupt(int irq, void *dev_id)
  * cyclecounter init routine and exits.
  */
 
-void fec_ptp_init(struct platform_device *pdev, int irq_idx)
+int fec_ptp_init(struct platform_device *pdev, int irq_idx)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct fec_enet_private *fep = netdev_priv(ndev);
@@ -779,26 +779,32 @@ void fec_ptp_init(struct platform_device *pdev, int irq_idx)
 		      HRTIMER_MODE_REL);
 
 	irq = platform_get_irq_byname_optional(pdev, "pps");
-	if (irq < 0)
+	if (irq < 0 && irq != -ENXIO)
+		return irq;
+	if (irq == -ENXIO) {
 		irq = platform_get_irq_optional(pdev, irq_idx);
-	/* Failure to get an irq is not fatal,
-	 * only the PTP_CLOCK_PPS clock events should stop
-	 */
-	if (irq >= 0) {
+		if (irq < 0 && irq != -ENXIO)
+			return irq;
+	}
+
+	if (irq > 0) {
 		ret = devm_request_irq(&pdev->dev, irq, fec_pps_interrupt,
 				       0, pdev->name, ndev);
 		if (ret < 0)
-			dev_warn(&pdev->dev, "request for pps irq failed(%d)\n",
-				 ret);
+			return ret;
 	}
 
 	fep->ptp_clock = ptp_clock_register(&fep->ptp_caps, &pdev->dev);
 	if (IS_ERR(fep->ptp_clock)) {
+		ret = PTR_ERR(fep->ptp_clock);
 		fep->ptp_clock = NULL;
 		dev_err(&pdev->dev, "ptp_clock_register failed\n");
+		return ret;
 	}
 
 	schedule_delayed_work(&fep->time_keep, HZ);
+
+	return 0;
 }
 
 void fec_ptp_save_state(struct fec_enet_private *fep)
-- 
2.43.0

RE: [PATCH net-next v2] net: fec: Propagate PTP initialization errors

[Wei Fang]

> Change fec_ptp_init() to return an error code instead of silently
> ignoring failures during PTP initialization.
> 
> The PPS IRQ is not required for the FEC/PTP functionality, so its
> absence should not make the probe fail. However, an unavailable
> optional IRQ should be distinguished from an actual error returned
> during the IRQ lookup.
> 
> If a platform does not support the PPS IRQ, it can omit the IRQ from
> its device tree and the optional lookup will return -ENXIO. Propagate
> other errors from the IRQ lookup instead of silently ignoring them.
> 
> Also propagate failures from devm_request_irq() and ptp_clock_register().
> 
> Update the function declaration in fec.h accordingly.
> 
> Found by manual code inspection.
> 
> Fixes: b86bcb299092 ("net: fec_ptp: Use platform_get_irq_xxx_optional() to
> avoid error message")

If this is a bug fix, it should target to net tree rather than net-next.

Re: [PATCH net-next v2] net: fec: Propagate PTP initialization errors

[Bui Duc Phuc]

Hi Wei,

Thanks for the feedback.

> >
> > Fixes: b86bcb299092 ("net: fec_ptp: Use platform_get_irq_xxx_optional() to
> > avoid error message")
>
> If this is a bug fix, it should target to net tree rather than net-next.
>


On a previous (different) patch with a Fixes tag, I was advised to
target 'net-next' instead,
so I submitted this one to 'net-next' first to be safe.

If you think this fix for commit b86bcb299092 belongs in 'net', please
let me know and
I will resubmit it as a v2 targeting the 'net' tree.

Best regards,
Phuc

Re: [PATCH net-next v2] net: fec: Propagate PTP initialization errors

[Andrew Lunn]

On Thu, Sep 03, 2026 at 09:29:11AM +0000, Wei Fang wrote:
> > Change fec_ptp_init() to return an error code instead of silently
> > ignoring failures during PTP initialization.
> > 
> > The PPS IRQ is not required for the FEC/PTP functionality, so its
> > absence should not make the probe fail. However, an unavailable
> > optional IRQ should be distinguished from an actual error returned
> > during the IRQ lookup.
> > 
> > If a platform does not support the PPS IRQ, it can omit the IRQ from
> > its device tree and the optional lookup will return -ENXIO. Propagate
> > other errors from the IRQ lookup instead of silently ignoring them.
> > 
> > Also propagate failures from devm_request_irq() and ptp_clock_register().
> > 
> > Update the function declaration in fec.h accordingly.
> > 
> > Found by manual code inspection.
> > 
> > Fixes: b86bcb299092 ("net: fec_ptp: Use platform_get_irq_xxx_optional() to
> > avoid error message")
> 
> If this is a bug fix, it should target to net tree rather than net-next.

https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

Stable rules say it must be a bug that bothers people. Does this
bother you?

       Andrew

RE: [PATCH net-next v2] net: fec: Propagate PTP initialization errors

[Wei Fang]

> > > Fixes: b86bcb299092 ("net: fec_ptp: Use platform_get_irq_xxx_optional() to
> > > avoid error message")
> >
> > If this is a bug fix, it should target to net tree rather than net-next.
> >
> 
> 
> On a previous (different) patch with a Fixes tag, I was advised to
> target 'net-next' instead,
> so I submitted this one to 'net-next' first to be safe.
> 
> If you think this fix for commit b86bcb299092 belongs in 'net', please
> let me know and
> I will resubmit it as a v2 targeting the 'net' tree.
> 

Is this a real bug that bothers people? or just an improvement?
For real bug, I think the patch should target to net tree.

Re: [PATCH net-next v2] net: fec: Propagate PTP initialization errors

[Bui Duc Phuc]

>
> Is this a real bug that bothers people? or just an improvement?
> For real bug, I think the patch should target to net tree.
>

I consider this a real bug rather than just an improvement, for two reasons.

First, commit b86bcb299092 was a bug fix and was also backported to stable.
This patch fixes the error handling in that change, which incorrectly
treats errors
other than -ENXIO as an absent optional IRQ. Therefore, I believe this
fix is also
relevant to stable kernels.

Second, the current implementation can silently ignore -EPROBE_DEFER and
continue probing instead of deferring. On an affected platform, this
can result in
the PPS IRQ not being set up, causing the corresponding PPS functionality to be
unavailable to the user.

I don't have hardware to reproduce this specific case, so the issue
was found by
code inspection rather than a user report.
If you think this fix is appropriate for the net tree, I'm happy to
retarget and resend it there.

Best regards,
Phuc

RE: [PATCH net-next v2] net: fec: Propagate PTP initialization errors

[Wei Fang]

> > Is this a real bug that bothers people? or just an improvement?
> > For real bug, I think the patch should target to net tree.
> >
> 
> I consider this a real bug rather than just an improvement, for two reasons.
> 
> First, commit b86bcb299092 was a bug fix and was also backported to stable.
> This patch fixes the error handling in that change, which incorrectly
> treats errors
> other than -ENXIO as an absent optional IRQ. Therefore, I believe this
> fix is also
> relevant to stable kernels.
> 
> Second, the current implementation can silently ignore -EPROBE_DEFER and
> continue probing instead of deferring. On an affected platform, this
> can result in
> the PPS IRQ not being set up, causing the corresponding PPS functionality to be
> unavailable to the user.
> 
> I don't have hardware to reproduce this specific case, so the issue
> was found by
> code inspection rather than a user report.
> If you think this fix is appropriate for the net tree, I'm happy to
> retarget and resend it there.
> 

It is a potential issue, so the target tree should be net.

Re: [PATCH net-next v2] net: fec: Propagate PTP initialization errors

[Bui Duc Phuc]

Hi Wei,

Thanks for the clarification.

>
> It is a potential issue, so the target tree should be net.
>

I will retarget the patch to the net tree and resend it.

Best regards,
Phuc

Re: [PATCH net-next v2] net: fec: Propagate PTP initialization errors

[sashiko-bot]

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

Pre-existing issues:
- [High] The device interrupt handlers are exposed to a use-after-free window on probe error paths because devm_request_irq() is used alongside manual free_netdev().
- [High] Use-after-free on perout_timer due to incorrect teardown ordering in fec_ptp_stop().
--

commit c7933bca59e467b625e0e61dff17b0ea0651dbf8
Author: bui duc phuc <phucduc.bui@gmail.com>
Date:   Thu Sep 3 09:24:30 2026 +0000

    net: fec: Propagate PTP initialization errors

    This commit modifies fec_ptp_init() to return an error code instead of
    silently ignoring failures. It adds error propagation for devm_request_irq()
    and ptp_clock_register() up through the probe function.

    Link v1: https://lore.kernel.org/all/20260820111416.97917-1-phucduc.bui@gmail.com/

> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 794ec427b0ee6..c6e29b5c2abb4 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -5384,8 +5384,11 @@ fec_probe(struct platform_device *pdev)
>  		goto failed_reset;
>  
>  	irq_cnt = fec_enet_get_irq_cnt(pdev);
> -	if (fep->bufdesc_ex)
> -		fec_ptp_init(pdev, irq_cnt);
> +	if (fep->bufdesc_ex) {
> +		ret = fec_ptp_init(pdev, irq_cnt);
> +		if (ret)
> +			goto failed_reset;

[Severity: High]
This isn't a bug introduced by this patch, but returning an error here
exposes a pre-existing use-after-free window in the error path.

Because both the PPS interrupt and MAC interrupts are registered using
devm_request_irq(), their lifetime is tied to the platform device. When we
jump to failed_reset, the cleanup code eventually calls free_netdev(ndev).

Since devm_request_irq() teardown happens via devres_release_all() only
after fec_probe() returns, the interrupts remain active while the netdevice
and driver private data are already freed. 

If an interrupt fires during this window, wouldn't the handlers
(fec_pps_interrupt or fec_enet_interrupt) dereference the freed ndev and
fep pointers? Should the driver avoid mixing devm_request_irq() with manual
free_netdev() to prevent this?

> +	}
>  
>  	ret = fec_enet_init(ndev);
>  	if (ret)

[ ... ]

> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
> index 56801c2009d59..557a76797ebaa 100644
> --- a/drivers/net/ethernet/freescale/fec_ptp.c
> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
[ ... ]
> @@ -779,26 +779,32 @@ void fec_ptp_init(struct platform_device *pdev, int irq_idx)
>  		      HRTIMER_MODE_REL);
>  
>  	irq = platform_get_irq_byname_optional(pdev, "pps");
> -	if (irq < 0)
> +	if (irq < 0 && irq != -ENXIO)
> +		return irq;
> +	if (irq == -ENXIO) {
>  		irq = platform_get_irq_optional(pdev, irq_idx);
> -	/* Failure to get an irq is not fatal,
> -	 * only the PTP_CLOCK_PPS clock events should stop
> -	 */
> -	if (irq >= 0) {
> +		if (irq < 0 && irq != -ENXIO)
> +			return irq;
> +	}
> +
> +	if (irq > 0) {
>  		ret = devm_request_irq(&pdev->dev, irq, fec_pps_interrupt,
>  				       0, pdev->name, ndev);
>  		if (ret < 0)
> -			dev_warn(&pdev->dev, "request for pps irq failed(%d)\n",
> -				 ret);
> +			return ret;
>  	}
>  
>  	fep->ptp_clock = ptp_clock_register(&fep->ptp_caps, &pdev->dev);
>  	if (IS_ERR(fep->ptp_clock)) {
> +		ret = PTR_ERR(fep->ptp_clock);
>  		fep->ptp_clock = NULL;
>  		dev_err(&pdev->dev, "ptp_clock_register failed\n");
> +		return ret;
>  	}
>  
>  	schedule_delayed_work(&fep->time_keep, HZ);
> +
> +	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but since this patch handles the setup of
the PTP clock, it's worth noting the teardown path has a potential
use-after-free.

In fec_ptp_stop(), the driver cancels fep->perout_timer before calling
ptp_clock_unregister(fep->ptp_clock):

fec_ptp_stop() {
    ...
    hrtimer_cancel(&fep->perout_timer);
    if (fep->ptp_clock)
        ptp_clock_unregister(fep->ptp_clock);
    ...
}

Because the PTP clock device is still registered with the subsystem while
the timer is cancelled, could a concurrent userspace thread issue a
PTP_CLK_REQ_PEROUT ioctl and re-arm the timer just before the netdevice is
freed by the driver remove function? 

If so, when the re-armed timer expires, its callback 
(fec_ptp_pps_perout_handler) would execute on freed memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903092430.354186-1-phucduc.bui@gmail.com?part=1

Re: [PATCH net-next v2] net: fec: Propagate PTP initialization errors

[Bui Duc Phuc]

>
> Pre-existing issues:
> - [High] The device interrupt handlers are exposed to a use-after-free window on probe error paths because devm_request_irq() is used alongside manual free_netdev().

This series fixes the error path when devm_request_irq() fails by
propagating the error to the probe function.
The reported race between the PPS interrupt handler and device
teardown is a pre-existing issue and is unrelated to
the error propagation changes in this series.
I would like to leave this issue out of the current series.

However, if we do want to address that race, I think a potential fix would be
switching from devm_request_irq() to manual request_irq() and
explicitly calling free_irq() during teardown before cleaning up the
associated resources.


> - [High] Use-after-free on perout_timer due to incorrect teardown ordering in fec_ptp_stop().
> --
>

This issue may already have been addressed by this patch:

https://lore.kernel.org/all/E1uydLH-000000061DM-2gcV@rmk-PC.armlinux.org.uk/

Re: [PATCH net-next v2] net: fec: Propagate PTP initialization errors

[Andrew Lunn]

> Second, the current implementation can silently ignore -EPROBE_DEFER and
> continue probing instead of deferring.

This is a stronger reason. You should mention it in the commit message
as part of your justification for net, rather than net-next.

	Andrew

Re: [PATCH net-next v2] net: fec: Propagate PTP initialization errors

[Bui Duc Phuc]

Hi Andrew,

Thank you for your feedback.

> > Second, the current implementation can silently ignore -EPROBE_DEFER and
> > continue probing instead of deferring.
>
> This is a stronger reason. You should mention it in the commit message
> as part of your justification for net, rather than net-next.
>

I will add this point to the commit message.

Best regards,
Phuc