Re: [PATCH v4] scsi: sd: mark disk dead on surprise host removal to avoid I/O hang

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: Data loss on orderly host teardown due to misidentified surprise removal
--

commit 9a3b48997b6a95bc55f9d45d1fe71786ff63438c
Author:  <hulianqin@vivo.com>

scsi: sd: mark disk dead on surprise host removal to avoid I/O hang

This commit adds a scsi_host_in_cancel() check in sd_remove() to mark the
disk dead before del_gendisk(). This is intended to skip sync_filesystem()
during surprise removals where the device can no longer handle I/O.

> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index b0d2542fd3172..00cf408513ce4 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -4270,6 +4270,16 @@ static void sd_remove(struct scsi_device *sdp)
>  
>  	scsi_autopm_get_device(sdkp->device);
>  
> +	/*
> +	 * If the host is being torn down (e.g. USB surprise removal),
> +	 * the device cannot handle I/O anymore.
> +	 * Mark the disk dead before del_gendisk() so that new I/O is
> +	 * rejected immediately and sync_filesystem() in del_gendisk()
> +	 * is skipped, avoiding a hang waiting for I/O that can never
> +	 * complete.
> +	 */
> +	if (scsi_host_in_cancel(sdkp->device->host))
> +		blk_mark_disk_dead(sdkp->disk);

[Severity: High]
Will this logic conflate all host removals with surprise removals?

The patch aims to skip sync_filesystem() only during surprise removals by
checking if the host is in SHOST_CANCEL. However, scsi_remove_host() in
drivers/scsi/hosts.c unconditionally transitions the host state to
SHOST_CANCEL for all host teardowns:

drivers/scsi/hosts.c:scsi_remove_host() {
    ...
    if (scsi_host_set_state(shost, SHOST_CANCEL))
    ...
}

Because SHOST_CANCEL is the standard teardown state for all removals,
including orderly ones like rmmod usb-storage or sysfs driver unbinding,
blk_mark_disk_dead() will set GD_DEAD for every host removal.

When del_gendisk() executes, it will bypass its own
blk_report_disk_dead(disk, false) because GD_DEAD is already set.

Could this skip sync_filesystem() entirely, causing all pending dirty page
cache data to be discarded instead of written back during an orderly unbind
or unload, resulting in data loss?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/TYUPR06MB6217570B9AFA4B5980FCCED0D2932@TYUPR06MB6217.apcprd06.prod.outlook.com?part=1