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

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 14/05/2026 11:00, 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>
> ---
> v2 -> v3:
> * fix code issue in v2
> v1 -> v2:
> * on v1 sashiko mentioned that some IRQs may fire after bp->ptp freed.
>    it looks like a false positive, because ptp_clock_unregister() will
>    explicitly disable all EXTTS channels before destroying ptp_clock
>    structure. But there is a possibility of timestampers being active
>    without PTP_PF_EXTTS function set. It can be done through sysfs files
>    and it will change pins configuration directly, but can only be done
>    for TS1..TS4. Disable them explicitly after detaching sysfs files.
> ---
> 
>   drivers/ptp/ptp_ocp.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> index beacc2ffb166..b3a9480b9bcb 100644
> --- a/drivers/ptp/ptp_ocp.c
> +++ b/drivers/ptp/ptp_ocp.c
> @@ -4867,6 +4867,18 @@ ptp_ocp_detach(struct ptp_ocp *bp)
>   	ptp_ocp_detach_sysfs(bp);
>   	ptp_ocp_attr_group_del(bp);
>   	timer_delete_sync(&bp->watchdog);
> +	/* Disable interrupts on all timestampers */
> +	if (bp->ts1)
> +		ptp_ocp_ts_enable(bp->ts1, 0, false);
> +	if (bp->ts2)
> +		ptp_ocp_ts_enable(bp->ts2, 0, false);
> +	if (bp->ts3)
> +		ptp_ocp_ts_enable(bp->ts3, 0, false);
> +	if (bp->ts4)
> +		ptp_ocp_ts_enable(bp->ts4, 0, false);
> +	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);
> @@ -4884,9 +4896,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);
>   }
>   

Looks like Sashiko still has a point of in-flight irq handler.
Going to send v4 with synchronize_irq() added to the disable path
of ptp_ocp_ts_enable()

---
pw-bot: cr