* [PATCH 2/3] scsi: fc_transport: Fix TOCTOU races and workqueue
@ 2026-04-09 15:12 Kyle Mahlkuch
2026-07-06 12:43 ` Hannes Reinecke
2026-09-01 10:43 ` Maram Srimannarayana Murthy
0 siblings, 2 replies; 3+ messages in thread
From: Kyle Mahlkuch @ 2026-04-09 15:12 UTC (permalink / raw)
To: linux-scsi, linux-kernel, paul.ely; +Cc: thinhtr
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 2/3] scsi: fc_transport: Fix TOCTOU races and workqueue
2026-04-09 15:12 [PATCH 2/3] scsi: fc_transport: Fix TOCTOU races and workqueue Kyle Mahlkuch
@ 2026-07-06 12:43 ` Hannes Reinecke
2026-09-01 10:43 ` Maram Srimannarayana Murthy
1 sibling, 0 replies; 3+ messages in thread
From: Hannes Reinecke @ 2026-07-06 12:43 UTC (permalink / raw)
To: Kyle Mahlkuch, linux-scsi, linux-kernel, paul.ely; +Cc: thinhtr
On 4/9/26 5:12 PM, Kyle Mahlkuch wrote:
> 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);
> }
>
This is wrong. The only way when this will become an issue is
if someone calls fc_queue_work() after the queue has been deleted,
ie when the Scsi Host is in the process of being shutdown.
And I would argue that it's a programming error if some driver would
scheduled workqueue elements at that time; the driver should know that
the host is about to be terminated, and should not try to schedule
anything here.
Seeing that every driver is using this sequence:
fc_remove_host()
scsi_remove_host()
and scsi_remove_host() is setting the 'SHOST_CANCEL' state
it would be far better to set the 'SHOST_CANCEL' state first and
then call fc_remove_host().
Then we can have an easy check in fc_queue_work() to disallow any
new elements when the host state is SHOST_CANCEL (or SHOST_DELETE).
I'll send a patch.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 2/3] scsi: fc_transport: Fix TOCTOU races and workqueue
2026-04-09 15:12 [PATCH 2/3] scsi: fc_transport: Fix TOCTOU races and workqueue Kyle Mahlkuch
2026-07-06 12:43 ` Hannes Reinecke
@ 2026-09-01 10:43 ` Maram Srimannarayana Murthy
1 sibling, 0 replies; 3+ messages in thread
From: Maram Srimannarayana Murthy @ 2026-09-01 10:43 UTC (permalink / raw)
To: Kyle Mahlkuch, linux-scsi, linux-kernel, paul.ely; +Cc: thinhtr
On 09/04/26 8:42 pm, Kyle Mahlkuch wrote:
> 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>
> ---
Tested-by: Maram Srimannarayana Murthy <msmurthy@linux.ibm.com>
Tested the complete 3-patch series on an IBM Power11 (ppc64le) server
equipped with an Emulex FC HBA.
The patches applied cleanly, and FC driver parameter validation testing
was executed continuously for 36 hours.
No crashes, hangs, or functional issues were observed during the test
period.
Thanks,
Maram Srimannarayana Murthy
> 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);
> }
> }
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-01 10:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-09 15:12 [PATCH 2/3] scsi: fc_transport: Fix TOCTOU races and workqueue Kyle Mahlkuch
2026-07-06 12:43 ` Hannes Reinecke
2026-09-01 10:43 ` Maram Srimannarayana Murthy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox