Re: [PATCH net 2/4] drop_monitor: use timer_shutdown_sync() to prevent timer rearming during teardown

[Hangbin Liu <hangbin.liu@linux.dev>]

On Thu, Sep 10, 2026 at 08:46:10PM +0000, Eric Dumazet wrote:
> In drop_monitor teardown paths (net_dm_trace_off_set(),
> net_dm_hw_monitor_stop(), and error unwind paths in net_dm_trace_on_set()
> and net_dm_hw_monitor_start()), per-CPU timers are stopped using
> timer_delete_sync() followed by cancel_work_sync().
> 
> However, there is a circular dependency between send_timer and
> dm_alert_work:
> 1) sched_send_work() (timer callback) schedules dm_alert_work.
> 2) send_dm_alert() / net_dm_hw_summary_work() calls reset_per_cpu_data()
>    or net_dm_hw_reset_per_cpu_data().
> 3) If memory allocation fails under memory pressure in the reset
>    function, it re-arms the timer via mod_timer(&data->send_timer, ...).
> 
> If dm_alert_work is running concurrently while timer_delete_sync()
> executes on another CPU, an allocation failure in the worker will
> re-arm the timer after timer_delete_sync() has already returned.
> Once cancel_work_sync() completes and module_put() is called, the timer
> remains active in the timer wheel. If the module is then unloaded, the
> timer will fire and execute sched_send_work() in freed memory,
> triggering a kernel panic / use-after-free.
> 
> Switch from timer_delete_sync() to timer_shutdown_sync(). This guarantees
> that any in-flight timer handler has finished and prevents subsequent
> re-arming attempts from running workers from succeeding. When monitoring
> is restarted later, timer_setup() is invoked, which cleanly
> re-initializes the timer.
> 
> Fixes: 9398e9c0b1d4 ("drop_monitor: Perform cleanup upon probe registration failure")

Nit, not sure if we need add

70c69274f354 ("drop_monitor: Initialize timer and work item upon tracing enable")
and 8e94c3bc922e ("drop_monitor: Allow user to start monitoring hardware drops")

Thanks
Hangbin

> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  net/core/drop_monitor.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
> index 018d19e3a71de0be50bcc5753ba65f715f678aaa..873155ca72432924322bb7961996dd3430d052bc 100644
> --- a/net/core/drop_monitor.c
> +++ b/net/core/drop_monitor.c
> @@ -1083,7 +1083,7 @@ static int net_dm_hw_monitor_start(struct netlink_ext_ack *extack)
>  		struct per_cpu_dm_data *hw_data = &per_cpu(dm_hw_cpu_data, cpu);
>  		struct sk_buff *skb;
>  
> -		timer_delete_sync(&hw_data->send_timer);
> +		timer_shutdown_sync(&hw_data->send_timer);
>  		cancel_work_sync(&hw_data->dm_alert_work);
>  		while ((skb = __skb_dequeue(&hw_data->drop_queue))) {
>  			struct devlink_trap_metadata *hw_metadata;
> @@ -1117,7 +1117,7 @@ static void net_dm_hw_monitor_stop(struct netlink_ext_ack *extack)
>  		struct per_cpu_dm_data *hw_data = &per_cpu(dm_hw_cpu_data, cpu);
>  		struct sk_buff *skb;
>  
> -		timer_delete_sync(&hw_data->send_timer);
> +		timer_shutdown_sync(&hw_data->send_timer);
>  		cancel_work_sync(&hw_data->dm_alert_work);
>  		while ((skb = __skb_dequeue(&hw_data->drop_queue))) {
>  			struct devlink_trap_metadata *hw_metadata;
> @@ -1179,7 +1179,7 @@ static int net_dm_trace_on_set(struct netlink_ext_ack *extack)
>  		struct per_cpu_dm_data *data = &per_cpu(dm_cpu_data, cpu);
>  		struct sk_buff *skb;
>  
> -		timer_delete_sync(&data->send_timer);
> +		timer_shutdown_sync(&data->send_timer);
>  		cancel_work_sync(&data->dm_alert_work);
>  		while ((skb = __skb_dequeue(&data->drop_queue)))
>  			consume_skb(skb);
> @@ -1207,7 +1207,7 @@ static void net_dm_trace_off_set(void)
>  		struct per_cpu_dm_data *data = &per_cpu(dm_cpu_data, cpu);
>  		struct sk_buff *skb;
>  
> -		timer_delete_sync(&data->send_timer);
> +		timer_shutdown_sync(&data->send_timer);
>  		cancel_work_sync(&data->dm_alert_work);
>  		while ((skb = __skb_dequeue(&data->drop_queue)))
>  			consume_skb(skb);
> -- 
> 2.55.0.1007.g17ff1f9808-goog
>