Re: [PATCH] scsi: virtio_scsi: set DRIVER_OK before registering the SCSI host

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Preexisting Use-After-Free in the SCSI error handler or block layer when `scsi_add_host()` fails after a concurrent sysfs scan.
--

--- Patch [1]: [PATCH] scsi: virtio_scsi: set DRIVER_OK before registering the SCSI host ---
commit 58209fcc60c7b4a593d3a3456eb44d5ff9a7db6b
Author: Jasper Wise <jaspwise@amazon.co.uk>

scsi: virtio_scsi: set DRIVER_OK before registering the SCSI host

This commit alters the virtio-scsi device initialization to set DRIVER_OK
prior to calling scsi_add_host(). It also updates the probe error handling
to ensure the device is correctly reset if host registration fails.

> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> --- a/drivers/scsi/virtio_scsi.c
> +++ b/drivers/scsi/virtio_scsi.c
[ ... ]
> @@ -991,7 +991,7 @@ static int virtscsi_probe(struct virtio_device *vdev)
>  	return 0;
>  
>  scsi_add_host_failed:
> -	vdev->config->del_vqs(vdev);
> +	virtscsi_remove_vqs(vdev);

[Severity: High]
This is a pre-existing issue, but could this error path trigger a
use-after-free if scsi_add_host() fails?

When scsi_add_host() runs, it calls scsi_add_host_with_dma(), which exposes
the scan sysfs attribute via device_add():

drivers/scsi/hosts.c:scsi_add_host_with_dma() {
    ...
    error = device_add(&shost->shost_dev);
    if (error)
        goto out_del_gendev;
    ...
}

If a subsequent initialization step inside device_add() fails,
scsi_add_host() will unwind and return an error.

If a concurrent write to the scan attribute happens before the unwind,
scsi_scan_host_selected() can create child SCSI targets. Because
scsi_add_host() does not reap these concurrently created children on its
error path, they remain orphaned.

If we proceed to free the virtqueues here via virtscsi_remove_vqs(),
could a later SCSI error handler invocation such as virtscsi_abort()
dereference the freed virtqueues?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813084405.610322-1-jaspwise@amazon.co.uk?part=1