[PATCH] ptp: ocp: fix resource freeing order

[PATCH] ptp: ocp: fix resource freeing order

[Vadim Fedorenko]

Commit a60fc3294a37 ("ptp: rework ptp_clock_unregister() to disable
events") added a call to ptp_disable_all_events() which changes the
configuration of pins if they support EXTTS events. In ptp_ocp_detach()
pins resources are freed before ptp_clock_unregister() and it leads to
use-after-free during driver removal. Fix it by changing the order of
free/unregister calls.

Fixes: a60fc3294a37 ("ptp: rework ptp_clock_unregister() to disable events")
Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
---
 drivers/ptp/ptp_ocp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index d88ab2f86b1b..16fd4804fc6a 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -4558,6 +4558,9 @@ ptp_ocp_detach(struct ptp_ocp *bp)
 	ptp_ocp_detach_sysfs(bp);
 	ptp_ocp_attr_group_del(bp);
 	timer_delete_sync(&bp->watchdog);
+	if (bp->ptp)
+		ptp_clock_unregister(bp->ptp);
+	kfree(bp->ptp_info.pin_config);
 	ptp_ocp_unregister_ext(bp->ts0);
 	ptp_ocp_unregister_ext(bp->ts1);
 	ptp_ocp_unregister_ext(bp->ts2);
@@ -4575,9 +4578,6 @@ ptp_ocp_detach(struct ptp_ocp *bp)
 		clk_hw_unregister_fixed_rate(bp->i2c_clk);
 	if (bp->n_irqs)
 		pci_free_irq_vectors(bp->pdev);
-	if (bp->ptp)
-		ptp_clock_unregister(bp->ptp);
-	kfree(bp->ptp_info.pin_config);
 	device_unregister(&bp->dev);
 }
 
-- 
2.47.3

Re: [PATCH] ptp: ocp: fix resource freeing order

[Vladimir Oltean]

On Wed, Mar 25, 2026 at 12:05:10PM +0000, Vadim Fedorenko wrote:
> Commit a60fc3294a37 ("ptp: rework ptp_clock_unregister() to disable
> events") added a call to ptp_disable_all_events() which changes the
> configuration of pins if they support EXTTS events. In ptp_ocp_detach()
> pins resources are freed before ptp_clock_unregister() and it leads to
> use-after-free during driver removal. Fix it by changing the order of
> free/unregister calls.
> 
> Fixes: a60fc3294a37 ("ptp: rework ptp_clock_unregister() to disable events")
> Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> ---
>  drivers/ptp/ptp_ocp.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> index d88ab2f86b1b..16fd4804fc6a 100644
> --- a/drivers/ptp/ptp_ocp.c
> +++ b/drivers/ptp/ptp_ocp.c
> @@ -4558,6 +4558,9 @@ ptp_ocp_detach(struct ptp_ocp *bp)
>  	ptp_ocp_detach_sysfs(bp);
>  	ptp_ocp_attr_group_del(bp);
>  	timer_delete_sync(&bp->watchdog);
> +	if (bp->ptp)
> +		ptp_clock_unregister(bp->ptp);
> +	kfree(bp->ptp_info.pin_config);
>  	ptp_ocp_unregister_ext(bp->ts0);
>  	ptp_ocp_unregister_ext(bp->ts1);
>  	ptp_ocp_unregister_ext(bp->ts2);
> @@ -4575,9 +4578,6 @@ ptp_ocp_detach(struct ptp_ocp *bp)
>  		clk_hw_unregister_fixed_rate(bp->i2c_clk);
>  	if (bp->n_irqs)
>  		pci_free_irq_vectors(bp->pdev);
> -	if (bp->ptp)
> -		ptp_clock_unregister(bp->ptp);
> -	kfree(bp->ptp_info.pin_config);
>  	device_unregister(&bp->dev);
>  }
>  
> -- 
> 2.47.3
>

The code wasn't safe even before the patch, since user space could have
emitted a PTP ioctl that passed into the driver in between the
ptp_ocp_unregister_ext() calls and the subsequent ptp_clock_unregister()
call. It's just that the patch you blame made it more obvious.

Re: [PATCH] ptp: ocp: fix resource freeing order

[Russell King (Oracle)]

On Wed, Mar 25, 2026 at 02:20:05PM +0200, Vladimir Oltean wrote:
> On Wed, Mar 25, 2026 at 12:05:10PM +0000, Vadim Fedorenko wrote:
> > Commit a60fc3294a37 ("ptp: rework ptp_clock_unregister() to disable
> > events") added a call to ptp_disable_all_events() which changes the
> > configuration of pins if they support EXTTS events. In ptp_ocp_detach()
> > pins resources are freed before ptp_clock_unregister() and it leads to
> > use-after-free during driver removal. Fix it by changing the order of
> > free/unregister calls.
> > 
> > Fixes: a60fc3294a37 ("ptp: rework ptp_clock_unregister() to disable events")
> > Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> > ---
> >  drivers/ptp/ptp_ocp.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> > index d88ab2f86b1b..16fd4804fc6a 100644
> > --- a/drivers/ptp/ptp_ocp.c
> > +++ b/drivers/ptp/ptp_ocp.c
> > @@ -4558,6 +4558,9 @@ ptp_ocp_detach(struct ptp_ocp *bp)
> >  	ptp_ocp_detach_sysfs(bp);
> >  	ptp_ocp_attr_group_del(bp);
> >  	timer_delete_sync(&bp->watchdog);
> > +	if (bp->ptp)
> > +		ptp_clock_unregister(bp->ptp);
> > +	kfree(bp->ptp_info.pin_config);
> >  	ptp_ocp_unregister_ext(bp->ts0);
> >  	ptp_ocp_unregister_ext(bp->ts1);
> >  	ptp_ocp_unregister_ext(bp->ts2);
> > @@ -4575,9 +4578,6 @@ ptp_ocp_detach(struct ptp_ocp *bp)
> >  		clk_hw_unregister_fixed_rate(bp->i2c_clk);
> >  	if (bp->n_irqs)
> >  		pci_free_irq_vectors(bp->pdev);
> > -	if (bp->ptp)
> > -		ptp_clock_unregister(bp->ptp);
> > -	kfree(bp->ptp_info.pin_config);
> >  	device_unregister(&bp->dev);
> >  }
> >  
> > -- 
> > 2.47.3
> >
> 
> The code wasn't safe even before the patch, since user space could have
> emitted a PTP ioctl that passed into the driver in between the
> ptp_ocp_unregister_ext() calls and the subsequent ptp_clock_unregister()
> call. It's just that the patch you blame made it more obvious.

Indeed.

We've been talking about "setup, then publish interfaces, unpublish
interfaces, tear down" for decades. I remember it coming up at the
Ottawa Linux Symposiums in the early 2000s when the driver model was
in its infancy. Yet, here we are in 2026, and the same problem
persists. :(

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!