Skip to content

Commit cfa7d9b

Browse files
stonezdmkuba-moo
authored andcommitted
cnic: Fix use-after-free bugs in cnic_delete_task
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, the flush_workqueue() in cnic_cm_stop_bnx2x_hw() 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 workqueue. 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_netdev_event() | cnic_stop_hw() | cnic_delete_task() cnic_cm_stop_bnx2x_hw() | ... cancel_delayed_work() | /* the queue_delayed_work() flush_workqueue() | executes after flush_workqueue()*/ | queue_delayed_work() cnic_free_dev(dev)//free | cnic_delete_task() //new instance | dev = cp->dev; //use Replace cancel_delayed_work() with cancel_delayed_work_sync() to ensure that the cyclic delayed work item is properly canceled and that any ongoing execution of the work item completes before the cnic_dev is deallocated. Furthermore, since cancel_delayed_work_sync() uses __flush_work(work, true) to synchronously wait for any currently executing instance of the work item to finish, the flush_workqueue() becomes redundant and should be removed. This bug was identified through static analysis. To reproduce the issue and validate the fix, I simulated the cnic PCI device in QEMU and introduced intentional delays — such as inserting calls to ssleep() within the cnic_delete_task() function — to increase the likelihood of triggering the bug. Fixes: fdf2408 ("cnic: Defer iscsi connection cleanup") Signed-off-by: Duoming Zhou <[email protected]> Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 3191df0 commit cfa7d9b

File tree

1 file changed

+1
-2
lines changed
  • drivers/net/ethernet/broadcom

1 file changed

+1
-2
lines changed

drivers/net/ethernet/broadcom/cnic.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4230,8 +4230,7 @@ static void cnic_cm_stop_bnx2x_hw(struct cnic_dev *dev)
42304230

42314231
cnic_bnx2x_delete_wait(dev, 0);
42324232

4233-
cancel_delayed_work(&cp->delete_task);
4234-
flush_workqueue(cnic_wq);
4233+
cancel_delayed_work_sync(&cp->delete_task);
42354234

42364235
if (atomic_read(&cp->iscsi_conn) != 0)
42374236
netdev_warn(dev->netdev, "%d iSCSI connections not destroyed\n",

0 commit comments

Comments
 (0)