[PATCH] scsi: core: Make SCSI host scan interruptible

[PATCH] scsi: core: Make SCSI host scan interruptible

[Bart Van Assche]

Writing to the sysfs "scan" attribute serializes SCSI host scans via
shost->scan_mutex. In virtualized environments such as virtio-scsi with
256 target IDs, scanning non-existent targets repeatedly allocates and
destroys temporary scsi_devices. Each iteration transitions the shared tag
queue state and freezes request queues, causing a full bus scan to take
tens of seconds.

When multiple processes initiate host scans concurrently, waiter tasks
sleep in TASK_UNINTERRUPTIBLE on shost->scan_mutex. If the waiting tasks
receive signals such as SIGKILL, they cannot wake up until the mutex is
acquired, easily exceeding the hung task timeout and triggering
khungtaskd warnings ("INFO: task hung in scsi_scan_host_selected").
Furthermore, once a task finally acquires the mutex, it continues to
perform the full scan even if a fatal signal is already pending.

Fix this by:
1. Use mutex_lock_interruptible() instead of mutex_lock() in
   scsi_scan_host_selected(), allowing waiting tasks to sleep in
   TASK_INTERRUPTIBLE and immediately wake up with -EINTR on signals.
2. Check signal_pending(current) in scsi_scan_host_selected(),
   scsi_scan_channel(), __scsi_scan_target(), scsi_report_lun_scan(),
   and scsi_sequential_lun_scan() to abort scanning early when a signal
   is delivered.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+1f678bd2872aefb11d8a@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1f678bd2872aefb11d8a
Assisted-by: Gemini
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_scan.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 96e4065ae8b5..ebe190f7e1c1 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -32,6 +32,7 @@
 #include <linux/blkdev.h>
 #include <linux/delay.h>
 #include <linux/kthread.h>
+#include <linux/sched/signal.h>
 #include <linux/spinlock.h>
 #include <linux/async.h>
 #include <linux/slab.h>
@@ -1415,11 +1416,14 @@ static void scsi_sequential_lun_scan(struct Scsi_Host *shost,
 	 * until we reach the max, or no LUN is found and we are not
 	 * sparse_lun.
 	 */
-	for (lun = 1; lun < max_dev_lun; ++lun)
+	for (lun = 1; lun < max_dev_lun; ++lun) {
+		if (signal_pending(current))
+			break;
 		if (scsi_probe_and_add_lun(shost, starget, lun, NULL, NULL,
 				rescan, NULL) != SCSI_SCAN_LUN_PRESENT &&
 		    !sparse_lun)
 			return;
+	}
 }
 
 /**
@@ -1591,6 +1595,9 @@ static int scsi_report_lun_scan(struct Scsi_Host *shost,
 	 * the header, so start at 1 and go up to and including num_luns.
 	 */
 	for (lunp = &lun_data[1]; lunp <= &lun_data[num_luns]; lunp++) {
+		if (signal_pending(current))
+			break;
+
 		lun = scsilun_to_int(lunp);
 
 		if (lun > sdev->host->max_lun) {
@@ -1804,6 +1811,8 @@ static void __scsi_scan_target(struct Scsi_Host *shost, struct device *parent,
 	 */
 	res = scsi_probe_and_add_lun(shost, starget, 0, &bflags, NULL, rescan,
 				     NULL);
+	if (signal_pending(current))
+		goto out_reap;
 	if (res == SCSI_SCAN_LUN_PRESENT || res == SCSI_SCAN_TARGET_PRESENT) {
 		if (scsi_report_lun_scan(shost, starget, bflags, rescan) != 0)
 			/*
@@ -1876,6 +1885,8 @@ static void scsi_scan_channel(struct Scsi_Host *shost, unsigned int channel,
 
 	if (id == SCAN_WILD_CARD)
 		for (id = 0; id < shost->max_id; ++id) {
+			if (signal_pending(current))
+				break;
 			/*
 			 * XXX adapter drivers when possible (FCP, iSCSI)
 			 * could modify max_id to match the current max,
@@ -1895,7 +1906,7 @@ static void scsi_scan_channel(struct Scsi_Host *shost, unsigned int channel,
 			__scsi_scan_target(shost, &shost->shost_gendev, channel,
 					order_id, lun, rescan);
 		}
-	else
+	else if (!signal_pending(current))
 		__scsi_scan_target(shost, &shost->shost_gendev, channel,
 				id, lun, rescan);
 }
@@ -1904,6 +1915,8 @@ int scsi_scan_host_selected(struct Scsi_Host *shost, unsigned int channel,
 			    unsigned int id, u64 lun,
 			    enum scsi_scan_mode rescan)
 {
+	int ret;
+
 	SCSI_LOG_SCAN_BUS(3, shost_printk (KERN_INFO, shost,
 		"%s: <%u:%u:%llu>\n",
 		__func__, channel, id, lun));
@@ -1913,23 +1926,30 @@ int scsi_scan_host_selected(struct Scsi_Host *shost, unsigned int channel,
 	    ((lun != SCAN_WILD_CARD) && (lun >= shost->max_lun)))
 		return -EINVAL;
 
-	mutex_lock(&shost->scan_mutex);
+	ret = mutex_lock_interruptible(&shost->scan_mutex);
+	if (ret)
+		return ret;
+
 	if (!shost->async_scan)
 		scsi_complete_async_scans();
 
-	if (scsi_host_scan_allowed(shost) && scsi_autopm_get_host(shost) == 0) {
+	if (!signal_pending(current) && scsi_host_scan_allowed(shost) &&
+	    scsi_autopm_get_host(shost) == 0) {
 		if (channel == SCAN_WILD_CARD)
 			for (channel = 0; channel <= shost->max_channel;
-			     channel++)
+			     channel++) {
+				if (signal_pending(current))
+					break;
 				scsi_scan_channel(shost, channel, id, lun,
 						  rescan);
+			}
 		else
 			scsi_scan_channel(shost, channel, id, lun, rescan);
 		scsi_autopm_put_host(shost);
 	}
 	mutex_unlock(&shost->scan_mutex);
 
-	return 0;
+	return signal_pending(current) ? -EINTR : 0;
 }
 
 static void scsi_sysfs_add_devices(struct Scsi_Host *shost)

Re: [PATCH] scsi: core: Make SCSI host scan interruptible

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Non-fatal signals and TIF_NOTIFY_SIGNAL silently abort SCSI scans due to incorrect use of signal_pending() and mutex_lock_interruptible().

Pre-existing issues:
- [High] Transport-backed sysfs scans still sleep in TASK_UNINTERRUPTIBLE on shost->scan_mutex.
--

commit 78930d642b0da5b5ffc8a1e28c1f7cf1e6e1e5b5
Author: Bart Van Assche <bvanassche@acm.org>

scsi: core: Make SCSI host scan interruptible

This commit modifies SCSI host scanning to use interruptible locking and
check for pending signals during scans. It aims to prevent hung task
warnings when tasks waiting on shost->scan_mutex receive fatal signals
like SIGKILL.

Link: https://syzkaller.appspot.com/bug?extid=1f678bd2872aefb11d8a

> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 4a4f11d2c7798..3161aa1bc8dcd 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c

[ ... ]

> @@ -1591,6 +1595,9 @@ static int scsi_report_lun_scan(struct Scsi_Host *shost,
>  	 * the header, so start at 1 and go up to and including num_luns.
>  	 */
>  	for (lunp = &lun_data[1]; lunp <= &lun_data[num_luns]; lunp++) {
> +		if (signal_pending(current))
> +			break;
> +

[Severity: High]
Can this silently abort LUN scanning on non-fatal signals?

If a non-fatal signal (like SIGCHLD) or TIF_NOTIFY_SIGNAL (used by
io_uring or task work) is delivered to the thread performing the scan,
signal_pending() returns true. This causes the loop in
scsi_report_lun_scan() to break and return success, leaving devices
undiscovered without reporting an error.

>  		lun = scsilun_to_int(lunp);
>  
>  		if (lun > sdev->host->max_lun) {

[ ... ]

> @@ -1913,23 +1926,30 @@ int scsi_scan_host_selected(struct Scsi_Host *shost, unsigned int channel,
>  	    ((lun != SCAN_WILD_CARD) && (lun >= shost->max_lun)))
>  		return -EINVAL;
>  
> -	mutex_lock(&shost->scan_mutex);
> +	ret = mutex_lock_interruptible(&shost->scan_mutex);
> +	if (ret)
> +		return ret;
> +

[Severity: High]
Are we risking silent discovery failures by using mutex_lock_interruptible()?

Since mutex_lock_interruptible() returns -EINTR on non-fatal signals,
it prematurely fails the scan lock acquisition.

Furthermore, do_scsi_scan_host() silently discards the -EINTR return value:

drivers/scsi/scsi_scan.c:do_scsi_scan_host() {
    ...
    scsi_scan_host_selected(shost, SCAN_WILD_CARD, SCAN_WILD_CARD,
                            SCAN_WILD_CARD, SCSI_SCAN_INITIAL);
    ...
}

Will this cause module loads to incorrectly succeed without discovering
attached devices if a non-fatal signal is pending during initialization?


[Severity: High]
This is a pre-existing issue, but transport-backed sysfs scans still seem
vulnerable to the original hung task issue this patch addresses.

Concurrent sysfs scans initiated by transport layers (FC, iSCSI, SAS) bypass
scsi_scan_host_selected() and block uninterruptibly in scsi_scan_target()
and __scsi_add_device():

drivers/scsi/scsi_scan.c:scsi_scan_target() {
    ...
    mutex_lock(&shost->scan_mutex);
    ...
}

Will tasks initiating these scans still exceed the hung task timeout if
fatal signals like SIGKILL are delivered while they wait on scan_mutex?

>  	if (!shost->async_scan)
>  		scsi_complete_async_scans();
>  
> -	if (scsi_host_scan_allowed(shost) && scsi_autopm_get_host(shost) == 0) {
> +	if (!signal_pending(current) && scsi_host_scan_allowed(shost) &&

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/74786683eadc0a32ab335373ead8dd76a9d5a342.1789157339.git.bvanassche@acm.org?part=1

Re: [PATCH] scsi: core: Make SCSI host scan interruptible

[Bart Van Assche]

On 9/11/26 1:23 PM, sashiko-bot@kernel.org wrote:
> [Severity: High]
> Can this silently abort LUN scanning on non-fatal signals?
> 
> If a non-fatal signal (like SIGCHLD) or TIF_NOTIFY_SIGNAL (used by
> io_uring or task work) is delivered to the thread performing the scan,
> signal_pending() returns true. This causes the loop in
> scsi_report_lun_scan() to break and return success, leaving devices
> undiscovered without reporting an error.

It doesn't make a difference how non-fatal signals are treated - if a
scan is interrupted, the scan will be incomplete.

> [Severity: High]
> This is a pre-existing issue, but transport-backed sysfs scans still seem
> vulnerable to the original hung task issue this patch addresses.
> 
> Concurrent sysfs scans initiated by transport layers (FC, iSCSI, SAS) bypass
> scsi_scan_host_selected() and block uninterruptibly in scsi_scan_target()
> and __scsi_add_device():
> 
> drivers/scsi/scsi_scan.c:scsi_scan_target() {
>      ...
>      mutex_lock(&shost->scan_mutex);
>      ...
> }
> 
> Will tasks initiating these scans still exceed the hung task timeout if
> fatal signals like SIGKILL are delivered while they wait on scan_mutex?

Transport layer scans not initiated from user space fall outside the
scope of this patch.

Bart.

Re: [PATCH] scsi: core: Make SCSI host scan interruptible

[Stanley Jhu]

On 9/11/26 1:11 PM, Bart Van Assche wrote:
> Writing to the sysfs "scan" attribute serializes SCSI host scans via
> shost->scan_mutex.

> -	mutex_lock(&shost->scan_mutex);
> +	ret = mutex_lock_interruptible(&shost->scan_mutex);
> +	if (ret)
> +		return ret;

You noted earlier on the thread that transport scans not initiated from
user space are out of scope. A write to the sysfs "scan" attribute is
always synchronous in the caller's task context, and on SAS, FC and
iSCSI hosts it keeps sleeping in TASK_UNINTERRUPTIBLE after this change:

- store_scan() -> scsi_scan() in scsi_sysfs.c reaches
   scsi_scan_host_selected() only when shost->transportt->user_scan is
   NULL.
- Those three transports each install one, and storvsc overrides the FC
   template with its own. All four reach scsi_scan_target(), which still
   takes scan_mutex with plain mutex_lock() and returns void.

> +	if (!signal_pending(current) && scsi_host_scan_allowed(shost) &&
> +	    scsi_autopm_get_host(shost) == 0) {

Conversely, initial host enumeration during driver probe should not be
aborted by user signals, yet this change makes it abortable whenever it
runs synchronously rather than on a kworker:

- scsi_prep_async_scan() returns NULL when scsi_scan_type is "sync",
   which is the built-in default when CONFIG_SCSI_SCAN_ASYNC=n.
- scsi_scan_host() then calls do_scsi_scan_host() synchronously in the
   caller's own task context, modprobe or a sysfs bind writer.
- do_scsi_scan_host() is void, so the return value is dropped and a
   signal on that task truncates enumeration with nothing surfacing it.

Both issues arise because signal_pending(current) cannot distinguish a
user-initiated rescan from driver probe enumeration. Would passing an
explicit flag or scan mode from user-initiated entry points
(store_scan() and procfs) be cleaner than inspecting current?

Re: [PATCH] scsi: core: Make SCSI host scan interruptible

[Bart Van Assche]

On 9/16/26 6:19 PM, Stanley Jhu wrote:
> Both issues arise because signal_pending(current) cannot distinguish a
> user-initiated rescan from driver probe enumeration. Would passing an
> explicit flag or scan mode from user-initiated entry points
> (store_scan() and procfs) be cleaner than inspecting current?

I'm waiting for others (long-time SCSI contributors) to comment on
whether or not they agree with making SCSI scanning interruptible.

Thanks,

Bart.

Re: [PATCH] scsi: core: Make SCSI host scan interruptible

[John Garry]

On 9/11/26 21:11, Bart Van Assche wrote:
> Writing to the sysfs "scan" attribute serializes SCSI host scans via
> shost->scan_mutex. In virtualized environments such as virtio-scsi with
> 256 target IDs, scanning non-existent targets repeatedly allocates and
> destroys temporary scsi_devices. Each iteration transitions the shared tag
> queue state and freezes request queues, causing a full bus scan to take
> tens of seconds.

So does this scan use all wildcard "- - -"?

> 
> When multiple processes initiate host scans concurrently, waiter tasks
> sleep in TASK_UNINTERRUPTIBLE on shost->scan_mutex. If the waiting tasks
> receive signals such as SIGKILL, they cannot wake up until the mutex is
> acquired, easily exceeding the hung task timeout and triggering
> khungtaskd warnings ("INFO: task hung in scsi_scan_host_selected").
> Furthermore, once a task finally acquires the mutex, it continues to
> perform the full scan even if a fatal signal is already pending.
> 
> Fix this by:
> 1. Use mutex_lock_interruptible() instead of mutex_lock() in
>     scsi_scan_host_selected(), allowing waiting tasks to sleep in
>     TASK_INTERRUPTIBLE and immediately wake up with -EINTR on signals.
> 2. Check signal_pending(current) in scsi_scan_host_selected(),
>     scsi_scan_channel(), __scsi_scan_target(), scsi_report_lun_scan(),
>     and scsi_sequential_lun_scan() to abort scanning early when a signal
>     is delivered.

If you want to be able to cancel a scan which is in progress, then why 
run it in the first place? I don't understand how this helps.