The original code uses cancel_delayed_work() in cnic_cm_stop_bnx2x_hw(),
which does not guarantee that the delayed work item 'delete_task' has
fully completed if it was already running. Additionally, the delayed work
item is cyclic, flush_workqueue() in cnic_cm_stop_bnx2x_hw() could not
prevent the new incoming ones. This leads to use-after-free scenarios
where the cnic_dev is deallocated by cnic_free_dev(), while delete_task
remains active and attempt to dereference cnic_dev in cnic_delete_task().

A typical race condition is illustrated below:

CPU 0 (cleanup)            | CPU 1 (delayed work callback)
cnic_stop_hw()             | cnic_delete_task()
  cnic_cm_stop_bnx2x_hw()  |
    cancel_delayed_work()  |   queue_delayed_work()
    flush_workqueue()      |
                           | cnic_delete_task() //new instance
cnic_free_dev(dev)//free   |
                           |   dev = cp->dev; //use

This is confirmed by a KASAN report:

BUG: KASAN: slab-use-after-free in __run_timer_base.part.0+0x7d7/0x8c0
Write of size 8 at addr ffff88800c0a55f0 by task kworker/u16:2/63
...
Call Trace:
 <IRQ>
 dump_stack_lvl+0x55/0x70
 print_report+0xcf/0x610
 ? __run_timer_base.part.0+0x7d7/0x8c0
 kasan_report+0xb8/0xf0
 ? __run_timer_base.part.0+0x7d7/0x8c0
 __run_timer_base.part.0+0x7d7/0x8c0
 ? rcu_sched_clock_irq+0xa57/0x27d0
 ? __pfx___run_timer_base.part.0+0x10/0x10
 ? __update_load_avg_cfs_rq+0x5f0/0xa50
 ? _raw_spin_lock_irq+0x80/0xe0
 ? __pfx__raw_spin_lock_irq+0x10/0x10
 ? tmigr_next_groupevt+0x99/0x140
 tmigr_handle_remote_up+0x603/0x7e0
 ? __pfx_tmigr_handle_remote_up+0x10/0x10
 ? sched_balance_trigger+0x199/0x9f0
 ? sched_tick+0x221/0x5a0
 ? _raw_spin_lock_irq+0x80/0xe0
 ? timerqueue_add+0x21b/0x320
 ? tick_nohz_handler+0x199/0x440
 ? __pfx_tmigr_handle_remote_up+0x10/0x10
 __walk_groups.isra.0+0x42/0x150
 tmigr_handle_remote+0x1f4/0x2e0
 ? __pfx_tmigr_handle_remote+0x10/0x10
 ? ktime_get+0x60/0x140
 ? lapic_next_event+0x11/0x20
 ? clockevents_program_event+0x1d4/0x2a0
 ? hrtimer_interrupt+0x322/0x780
 handle_softirqs+0x16a/0x550
 irq_exit_rcu+0xaf/0xe0
 sysvec_apic_timer_interrupt+0x70/0x80
 </IRQ>
...
Allocated by task 141:
 kasan_save_stack+0x24/0x50
 kasan_save_track+0x14/0x30
 __kasan_kmalloc+0x7f/0x90
 cnic_alloc_dev.isra.0+0x40/0x310
 is_cnic_dev+0x795/0x11e0
 cnic_netdev_event+0xde/0xd30
 notifier_call_chain+0xc0/0x280
 register_netdevice+0xfb5/0x16c0
 register_netdev+0x1b/0x40
...
Freed by task 63:
 kasan_save_stack+0x24/0x50
 kasan_save_track+0x14/0x30
 kasan_save_free_info+0x3a/0x60
 __kasan_slab_free+0x3f/0x50
 kfree+0x137/0x370
 cnic_netdev_event+0x972/0xd30
...

Replace cancel_delayed_work() with cancel_delayed_work_sync() to ensure
that the delayed work item is properly canceled and any executing delayed
work has finished before the cnic_dev is deallocated.

Fixes: fdf24086f475 ("cnic: Defer iscsi connection cleanup")
Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
---
 drivers/net/ethernet/broadcom/cnic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/cnic.c b/drivers/net/ethernet/broadcom/cnic.c
index a9040c42d2ff..73dd7c25d89e 100644
--- a/drivers/net/ethernet/broadcom/cnic.c
+++ b/drivers/net/ethernet/broadcom/cnic.c
@@ -4230,7 +4230,7 @@ static void cnic_cm_stop_bnx2x_hw(struct cnic_dev *dev)
 
 	cnic_bnx2x_delete_wait(dev, 0);
 
-	cancel_delayed_work(&cp->delete_task);
+	cancel_delayed_work_sync(&cp->delete_task);
 	flush_workqueue(cnic_wq);
 
 	if (atomic_read(&cp->iscsi_conn) != 0)
-- 
2.34.1

On Sun, 14 Sep 2025 11:43:35 +0800 Duoming Zhou wrote:
> The original code uses cancel_delayed_work() in cnic_cm_stop_bnx2x_hw(),
> which does not guarantee that the delayed work item 'delete_task' has
> fully completed if it was already running. Additionally, the delayed work
> item is cyclic, flush_workqueue() in cnic_cm_stop_bnx2x_hw() could not
> prevent the new incoming ones. This leads to use-after-free scenarios
> where the cnic_dev is deallocated by cnic_free_dev(), while delete_task
> remains active and attempt to dereference cnic_dev in cnic_delete_task().

[snip]

> Replace cancel_delayed_work() with cancel_delayed_work_sync() to ensure
> that the delayed work item is properly canceled and any executing delayed
> work has finished before the cnic_dev is deallocated.

Have you tested this on real HW? Please always include information on
how you discovered the problem and whether you managed to test the fix.

> Fixes: fdf24086f475 ("cnic: Defer iscsi connection cleanup")
> Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>

>  	cnic_bnx2x_delete_wait(dev, 0);
>  
> -	cancel_delayed_work(&cp->delete_task);
> +	cancel_delayed_work_sync(&cp->delete_task);
>  	flush_workqueue(cnic_wq);

AFAICT your patch is a nop, doubt this if fixing anything

>  	if (atomic_read(&cp->iscsi_conn) != 0)
-- 
pw-bot: cr
pv-bot: s

On Mon, 15 Sep 2025 18:22:35 -0700 Jakub Kicinski wrote:
> > The original code uses cancel_delayed_work() in cnic_cm_stop_bnx2x_hw(),
> > which does not guarantee that the delayed work item 'delete_task' has
> > fully completed if it was already running. Additionally, the delayed work
> > item is cyclic, flush_workqueue() in cnic_cm_stop_bnx2x_hw() could not
> > prevent the new incoming ones. This leads to use-after-free scenarios
> > where the cnic_dev is deallocated by cnic_free_dev(), while delete_task
> > remains active and attempt to dereference cnic_dev in cnic_delete_task().
> 
> [snip]
> 
> > Replace cancel_delayed_work() with cancel_delayed_work_sync() to ensure
> > that the delayed work item is properly canceled and any executing delayed
> > work has finished before the cnic_dev is deallocated.
> 
> Have you tested this on real HW? Please always include information on
> how you discovered the problem and whether you managed to test the fix.

To reproduce the issue, I emulated the cnic device in QEMU and manually
triggered the problem by introducing delays, such as calls to ssleep(), 
within the cnic_delete_task() function.

While the delayed work was executing, cancel_delayed_work() failed to 
terminate it. Furthermore, since cnic_delete_task() is a recurring
delayed work item, flush_workqueue() only blocks and waits for work
items that were already queued to the workqueue prior to its invocation. 
Any work items submitted after flush_workqueue() is called are not
included in the set of tasks that the flush operation awaits. This
means that after the cyclic work items have finished executing, 
a delayed work item may still exist in the work queue.

You can see the detail in the following link:
https://elixir.bootlin.com/linux/v6.17-rc6/source/kernel/workqueue.c#L3937

Furthermore, I wrote a kernel module to test whether the combination of
cancel_delayed_work() and flush_workqueue() can safely terminate recurring
delayed work items. The result is negative, indicating that the aforementioned
combination carries potential risks.

The cancel_delayed_work_sync calls __cancel_work(work, ... | WORK_CANCEL_DISABLE)
to attempt to remove the work item from the queue and sets the WORK_CANCEL_DISABLE
flag, preventing the work item from being executed again. Meanwhile, it uses
__flush_work(work, true) to perform a synchronous operation, waiting for any
currently executing work item to finish running.

You can see the detail in the following link:
https://elixir.bootlin.com/linux/v6.17-rc6/source/kernel/workqueue.c#L4348

> > Fixes: fdf24086f475 ("cnic: Defer iscsi connection cleanup")
> > Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
> 
> >  	cnic_bnx2x_delete_wait(dev, 0);
> >  
> > -	cancel_delayed_work(&cp->delete_task);
> > +	cancel_delayed_work_sync(&cp->delete_task);
> >  	flush_workqueue(cnic_wq);
> 
> AFAICT your patch is a nop, doubt this if fixing anything

This patch is not a nop, although the probability of triggering
this issue is low, this patch indeed fixes the underlying
problem.

Best regards,
Duoming Zhou