[PATCH v2] ibmvscsi: implement vio driver shutdown call back to quiesce ibmvscsi

[PATCH v2] ibmvscsi: implement vio driver shutdown call back to quiesce ibmvscsi

[Tyrel Datwyler]

Currently during kexec the vio driver implementation calls a drivers
remove function as a big hammer when shutdown isn't implemented. This
results in filesystem errors in the log as a result of the block device
being removed while the filesystem is still mounted. Rectify this noise
by quiescing the ibmvscsi driver and stopping short of tearing down the
full host.

Move quiesce calls to their own helper function. Add a driver shutdown
call back and implement with call to ibmvscsi_quiesce such that any
outstanding commands are purged and failed back to the midlayer with
DID_REQUEUE. Update ibmvscsi_remove to also use the new ibmvscsi_quiesce
and failback outstanding commands with DID_ERROR prior to full transport
teardown.

Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
Changes since v1:
- Only call scsi_block_requests in ibmvscsi_shutdown
- set_request_limit to 0 during quiesce to fail any new submissions

 drivers/scsi/ibmvscsi/ibmvscsi.c | 33 +++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index 4ecd3db08875..576f784b6677 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -2341,26 +2341,40 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const struct vio_device_id *id)
 	return -1;
 }
 
-static void ibmvscsi_remove(struct vio_dev *vdev)
+static void ibmvscsi_quiesce(struct ibmvscsi_host_data *hostdata, int scsi_error_code)
 {
-	struct ibmvscsi_host_data *hostdata = dev_get_drvdata(&vdev->dev);
-
-	srp_remove_host(hostdata->host);
-	scsi_remove_host(hostdata->host);
-
-	purge_requests(hostdata, DID_ERROR);
-	release_event_pool(&hostdata->pool, hostdata);
+	ibmvscsi_set_request_limit(hostdata, 0);
+	kthread_stop(hostdata->work_thread);
 
 	ibmvscsi_release_crq_queue(&hostdata->queue, hostdata,
 					max_events);
 
-	kthread_stop(hostdata->work_thread);
+	purge_requests(hostdata, scsi_error_code);
+
+	release_event_pool(&hostdata->pool, hostdata);
 	unmap_persist_bufs(hostdata);
+}
+
+static void ibmvscsi_shutdown(struct vio_dev *vdev)
+{
+	struct ibmvscsi_host_data *hostdata = dev_get_drvdata(&vdev->dev);
+
+	scsi_block_requests(hostdata->host);
+	ibmvscsi_quiesce(hostdata, DID_REQUEUE);
+}
+
+static void ibmvscsi_remove(struct vio_dev *vdev)
+{
+	struct ibmvscsi_host_data *hostdata = dev_get_drvdata(&vdev->dev);
+
+	ibmvscsi_quiesce(hostdata, DID_ERROR);
 
 	spin_lock(&ibmvscsi_driver_lock);
 	list_del(&hostdata->host_list);
 	spin_unlock(&ibmvscsi_driver_lock);
 
+	srp_remove_host(hostdata->host);
+	scsi_remove_host(hostdata->host);
 	scsi_host_put(hostdata->host);
 }
 
@@ -2398,6 +2412,7 @@ static struct vio_driver ibmvscsi_driver = {
 	.id_table = ibmvscsi_device_table,
 	.probe = ibmvscsi_probe,
 	.remove = ibmvscsi_remove,
+	.shutdown = ibmvscsi_shutdown,
 	.get_desired_dma = ibmvscsi_get_desired_dma,
 	.name = "ibmvscsi",
 	.pm = &ibmvscsi_pm_ops,
-- 
2.55.0

Re: [PATCH v2] ibmvscsi: implement vio driver shutdown call back to quiesce ibmvscsi

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: ibmvscsi: Use-After-Free in queuecommand due to inverted teardown
--

commit cb6df094529f08b7b5070579476f4f00c65a9dfd
Author: Tyrel Datwyler <tyreld@linux.ibm.com>

ibmvscsi: implement vio driver shutdown call back to quiesce ibmvscsi

Currently during kexec the vio driver implementation calls a drivers
remove function as a big hammer when shutdown isn't implemented. This
results in filesystem errors in the log as a result of the block device
being removed while the filesystem is still mounted. Rectify this noise
by quiescing the ibmvscsi driver and stopping short of tearing down the
full host.

> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
> index 609bda730b3a8..a8859cd603824 100644
> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c

[ ... ]

> +static void ibmvscsi_shutdown(struct vio_dev *vdev)
> +{
> +	struct ibmvscsi_host_data *hostdata = dev_get_drvdata(&vdev->dev);
> +
> +	scsi_block_requests(hostdata->host);
> +	ibmvscsi_quiesce(hostdata, DID_REQUEUE);
> +}

[Severity: High]
Can this create a use-after-free race condition with in-flight commands?

While scsi_block_requests() prevents new requests, it does not flush or wait for
in-flight commands. Calling ibmvscsi_quiesce() right after destroys the event
pool via release_event_pool(), which frees pool->events.

Any concurrent midlayer commands already in progress could attempt to access the
freed event pool.

> +
> +static void ibmvscsi_remove(struct vio_dev *vdev)
> +{
> +	struct ibmvscsi_host_data *hostdata = dev_get_drvdata(&vdev->dev);
> +
> +	ibmvscsi_quiesce(hostdata, DID_ERROR);
>  
>  	spin_lock(&ibmvscsi_driver_lock);
>  	list_del(&hostdata->host_list);
>  	spin_unlock(&ibmvscsi_driver_lock);
>  
> +	srp_remove_host(hostdata->host);
> +	scsi_remove_host(hostdata->host);

[Severity: High]
Does reordering these calls cause a deterministic use-after-free during unbind?

By calling ibmvscsi_quiesce() first, the event pool is destroyed and
pool->events is freed.

When scsi_remove_host() is subsequently called, it removes SCSI devices, which
synchronously dispatches a final SYNCHRONIZE CACHE command via the block layer.

This command reaches ibmvscsi_queuecommand_lck(), which attempts to allocate an
event struct from the freed pool:

drivers/scsi/ibmvscsi/ibmvscsi.c:get_event_struct() {
    ...
		if (!atomic_dec_if_positive(&pool->events[offset].free)) {
    ...
}

This will dereference the already freed pool->events array before any request
limits are checked.

>  	scsi_host_put(hostdata->host);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916213359.2144094-1-tyreld@linux.ibm.com?part=1