ptp: ocp: fix use-after-free bugs causing by ptp_ocp_watchdog

[PATCH net] ptp: ocp: fix use-after-free bugs causing by ptp_ocp_watchdog

Posted by Duoming Zhou 5 months, 1 week ago

The ptp_ocp_detach() only shuts down the watchdog timer if it is
pending. However, if the timer handler is already running, the
timer_delete_sync() is not called. This leads to race conditions
where the devlink that contains the ptp_ocp is deallocated while
the timer handler is still accessing it, resulting in use-after-free
bugs. The following details one of the race scenarios.

(thread 1)                           | (thread 2)
ptp_ocp_remove()                     |
  ptp_ocp_detach()                   | ptp_ocp_watchdog()
    if (timer_pending(&bp->watchdog))|   bp = timer_container_of()
      timer_delete_sync()            |
                                     |
  devlink_free(devlink) //free       |
                                     |   bp-> //use

Resolve this by unconditionally calling timer_delete_sync() to ensure
the timer is reliably deactivated, preventing any access after free.

Fixes: 773bda964921 ("ptp: ocp: Expose various resources on the timecard.")
Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
---
 drivers/ptp/ptp_ocp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index d39073dc4072..4e1286ce05c9 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -4557,8 +4557,7 @@ ptp_ocp_detach(struct ptp_ocp *bp)
 	ptp_ocp_debugfs_remove_device(bp);
 	ptp_ocp_detach_sysfs(bp);
 	ptp_ocp_attr_group_del(bp);
-	if (timer_pending(&bp->watchdog))
-		timer_delete_sync(&bp->watchdog);
+	timer_delete_sync(&bp->watchdog);
 	if (bp->ts0)
 		ptp_ocp_unregister_ext(bp->ts0);
 	if (bp->ts1)
-- 
2.34.1

Re: [PATCH net] ptp: ocp: fix use-after-free bugs causing by ptp_ocp_watchdog

Posted by Vadim Fedorenko 5 months, 1 week ago

On 28/08/2025 09:29, Duoming Zhou wrote:
> The ptp_ocp_detach() only shuts down the watchdog timer if it is
> pending. However, if the timer handler is already running, the
> timer_delete_sync() is not called. This leads to race conditions
> where the devlink that contains the ptp_ocp is deallocated while
> the timer handler is still accessing it, resulting in use-after-free
> bugs. The following details one of the race scenarios.
> 
> (thread 1)                           | (thread 2)
> ptp_ocp_remove()                     |
>    ptp_ocp_detach()                   | ptp_ocp_watchdog()
>      if (timer_pending(&bp->watchdog))|   bp = timer_container_of()
>        timer_delete_sync()            |
>                                       |
>    devlink_free(devlink) //free       |
>                                       |   bp-> //use
> 
> Resolve this by unconditionally calling timer_delete_sync() to ensure
> the timer is reliably deactivated, preventing any access after free.
> 
> Fixes: 773bda964921 ("ptp: ocp: Expose various resources on the timecard.")
> Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

> ---
>   drivers/ptp/ptp_ocp.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> index d39073dc4072..4e1286ce05c9 100644
> --- a/drivers/ptp/ptp_ocp.c
> +++ b/drivers/ptp/ptp_ocp.c
> @@ -4557,8 +4557,7 @@ ptp_ocp_detach(struct ptp_ocp *bp)
>   	ptp_ocp_debugfs_remove_device(bp);
>   	ptp_ocp_detach_sysfs(bp);
>   	ptp_ocp_attr_group_del(bp);
> -	if (timer_pending(&bp->watchdog))
> -		timer_delete_sync(&bp->watchdog);
> +	timer_delete_sync(&bp->watchdog);
>   	if (bp->ts0)
>   		ptp_ocp_unregister_ext(bp->ts0);
>   	if (bp->ts1)