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

[sashiko-bot@kernel.org]

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