diff mbox series

[v2,net-next] qed: Fix race condition between scheduling and destroying the slowpath workqueue

Message ID 20200325205043.23157-1-ybason@marvell.com
State Accepted
Delegated to: David Miller
Headers show
Series [v2,net-next] qed: Fix race condition between scheduling and destroying the slowpath workqueue | expand

Commit Message

Yuval Basson March 25, 2020, 8:50 p.m. UTC 
Calling queue_delayed_work concurrently with
destroy_workqueue might race to an unexpected outcome -
scheduled task after wq is destroyed or other resources
(like ptt_pool) are freed (yields NULL pointer dereference).
cancel_delayed_work prevents the race by cancelling
the timer triggered for scheduling a new task.

Fixes: 59ccf86fe ("qed: Add driver infrastucture for handling mfw requests")
Signed-off-by: Denis Bolotin <dbolotin@marvell.com>
Signed-off-by: Michal Kalderon <mkalderon@marvell.com>
Signed-off-by: Yuval Basson <ybason@marvell.com>
---

Changes in v2:
- This patch is sent as a stand-alone after being part of a patch series where the first
  two patches were dropped https://www.spinics.net/lists/netdev/msg639940.html.

 drivers/net/ethernet/qlogic/qed/qed_main.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

Comments

David Miller March 30, 2020, 4:50 a.m. UTC | #1 
From: Yuval Basson <ybason@marvell.com>
Date: Wed, 25 Mar 2020 22:50:43 +0200

> Calling queue_delayed_work concurrently with
> destroy_workqueue might race to an unexpected outcome -
> scheduled task after wq is destroyed or other resources
> (like ptt_pool) are freed (yields NULL pointer dereference).
> cancel_delayed_work prevents the race by cancelling
> the timer triggered for scheduling a new task.
> 
> Fixes: 59ccf86fe ("qed: Add driver infrastucture for handling mfw requests")
> Signed-off-by: Denis Bolotin <dbolotin@marvell.com>
> Signed-off-by: Michal Kalderon <mkalderon@marvell.com>
> Signed-off-by: Yuval Basson <ybason@marvell.com>

Applied.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index 2c189c6..96356e8 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -1087,9 +1087,6 @@  static void qed_update_pf_params(struct qed_dev *cdev,
 #define QED_PERIODIC_DB_REC_INTERVAL_MS		100
 #define QED_PERIODIC_DB_REC_INTERVAL \
 	msecs_to_jiffies(QED_PERIODIC_DB_REC_INTERVAL_MS)
-#define QED_PERIODIC_DB_REC_WAIT_COUNT		10
-#define QED_PERIODIC_DB_REC_WAIT_INTERVAL \
-	(QED_PERIODIC_DB_REC_INTERVAL_MS / QED_PERIODIC_DB_REC_WAIT_COUNT)
 
 static int qed_slowpath_delayed_work(struct qed_hwfn *hwfn,
 				     enum qed_slowpath_wq_flag wq_flag,
@@ -1123,7 +1120,7 @@  void qed_periodic_db_rec_start(struct qed_hwfn *p_hwfn)
 
 static void qed_slowpath_wq_stop(struct qed_dev *cdev)
 {
-	int i, sleep_count = QED_PERIODIC_DB_REC_WAIT_COUNT;
+	int i;
 
 	if (IS_VF(cdev))
 		return;
@@ -1135,13 +1132,7 @@  static void qed_slowpath_wq_stop(struct qed_dev *cdev)
 		/* Stop queuing new delayed works */
 		cdev->hwfns[i].slowpath_wq_active = false;
 
-		/* Wait until the last periodic doorbell recovery is executed */
-		while (test_bit(QED_SLOWPATH_PERIODIC_DB_REC,
-				&cdev->hwfns[i].slowpath_task_flags) &&
-		       sleep_count--)
-			msleep(QED_PERIODIC_DB_REC_WAIT_INTERVAL);
-
-		flush_workqueue(cdev->hwfns[i].slowpath_wq);
+		cancel_delayed_work(&cdev->hwfns[i].slowpath_task);
 		destroy_workqueue(cdev->hwfns[i].slowpath_wq);
 	}
 }