[PATCH 2/3] scsi: fc_transport: Fix TOCTOU races and workqueue

[PATCH 2/3] scsi: fc_transport: Fix TOCTOU races and workqueue

[Kyle Mahlkuch]

Fix the TOCTOU races in workqueue access, use READ_ONCE() in
fc_queue_work(), fc_flush_work(), fc_queue_devloss_work(), and
fc_flush_devloss().

The workqueue destruction in fc_remove_host() uses WRITE_ONCE() to set
the pointer to NULL to prevents new work, flushing the work queued
before NULL, then safely destroying it.

Signed-off-by: Thinh Tran <thinhtr@linux.ibm.com>
Signed-off-by: Kyle Mahlkuch <kmahlkuc@linux.ibm.com>
---
  drivers/scsi/scsi_transport_fc.c | 36 ++++++++++++++++++++++----------
  1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/scsi_transport_fc.c 
b/drivers/scsi/scsi_transport_fc.c
index 3a821afee9bc..123b22b52640 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -2774,16 +2774,18 @@ EXPORT_SYMBOL(fc_release_transport);
  static int
  fc_queue_work(struct Scsi_Host *shost, struct work_struct *work)
  {
-	if (unlikely(!fc_host_work_q(shost))) {
+	struct workqueue_struct *wq = READ_ONCE(fc_host_work_q(shost));
+
+	if (unlikely(!wq)) {
  		printk(KERN_ERR
  			"ERROR: FC host '%s' attempted to queue work, "
  			"when no workqueue created.\n", shost->hostt->name);
  		dump_stack();
-
  		return -EINVAL;
  	}

-	return queue_work(fc_host_work_q(shost), work);
+	/* Use local copy to prevent TOCTOU race */
+	return queue_work(wq, work);
  }

  /**
@@ -2793,7 +2795,9 @@ fc_queue_work(struct Scsi_Host *shost, struct 
work_struct *work)
  static void
  fc_flush_work(struct Scsi_Host *shost)
  {
-	if (!fc_host_work_q(shost)) {
+	struct workqueue_struct *wq = READ_ONCE(fc_host_work_q(shost));
+
+	if (!wq) {
  		printk(KERN_ERR
  			"ERROR: FC host '%s' attempted to flush work, "
  			"when no workqueue created.\n", shost->hostt->name);
@@ -2801,7 +2805,8 @@ fc_flush_work(struct Scsi_Host *shost)
  		return;
  	}

-	flush_workqueue(fc_host_work_q(shost));
+	/* Use local copy to prevent TOCTOU race */
+	flush_workqueue(wq);
  }

  /**
@@ -2818,16 +2823,18 @@ static int
  fc_queue_devloss_work(struct Scsi_Host *shost, struct fc_rport *rport,
  		      struct delayed_work *work, unsigned long delay)
  {
-	if (unlikely(!rport->devloss_work_q)) {
+	struct workqueue_struct *wq = READ_ONCE(rport->devloss_work_q);
+
+	if (unlikely(!wq)) {
  		printk(KERN_ERR
  			"ERROR: FC host '%s' attempted to queue work, "
  			"when no workqueue created.\n", shost->hostt->name);
  		dump_stack();
-
  		return -EINVAL;
  	}

-	return queue_delayed_work(rport->devloss_work_q, work, delay);
+	/* Use local copy to prevent TOCTOU race */
+	return queue_delayed_work(wq, work, delay);
  }

  /**
@@ -2838,7 +2845,9 @@ fc_queue_devloss_work(struct Scsi_Host *shost, 
struct fc_rport *rport,
  static void
  fc_flush_devloss(struct Scsi_Host *shost, struct fc_rport *rport)
  {
-	if (unlikely(!rport->devloss_work_q)) {
+	struct workqueue_struct *wq = READ_ONCE(rport->devloss_work_q);
+
+	if (unlikely(!wq)) {
  		printk(KERN_ERR
  			"ERROR: FC host '%s' attempted to flush work, "
  			"when no workqueue created.\n", shost->hostt->name);
@@ -2846,7 +2855,7 @@ fc_flush_devloss(struct Scsi_Host *shost, struct 
fc_rport *rport)
  		return;
  	}

-	flush_workqueue(rport->devloss_work_q);
+	flush_workqueue(wq);
  }


@@ -2905,7 +2914,12 @@ fc_remove_host(struct Scsi_Host *shost)
  	/* flush all stgt delete, and rport delete work items, then kill it  */
  	if (fc_host->work_q) {
  		work_q = fc_host->work_q;
-		fc_host->work_q = NULL;
+		/* Prevent new work from being queued by setting work_q to NULL */
+		WRITE_ONCE(fc_host->work_q, NULL);
+		/* Ensures NULL is visible to other CPUs before flush */
+		smp_mb();
+		/* Flush any work that was queued before NULL assignment */
+		flush_workqueue(work_q);
  		destroy_workqueue(work_q);
  	}
  }
-- 
2.52.0