Re: [PATCH v5 33/40] scsi: storvsc: use 16-bits defined sense codes

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free of a destroyed workqueue in storvsc_remove().
--

commit 78aa8a5b1c990af91c1c28201470aeedf9180e53
Author: Damien Le Moal <dlemoal@kernel.org>
Subject: scsi: storvsc: use 16-bits defined sense codes

This commit refactors the storvsc driver to use the 16-bit sense_code field
of struct scsi_sense_hdr and replaces hard-coded additional sense codes
with enum values. No functional change is intended.

> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index ef3cd3bc7390e..625db97d1a03e 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
[ ... ]
> @@ -1049,8 +1049,12 @@ static void storvsc_handle_error(struct vmscsi_request *vm_srb,
>  			 * want scsi_report_sense() to output a message
>  			 * that a sysadmin wouldn't know what to do with.
>  			 */
> -			if ((asc == 0x3f) && (ascq != 0x03) &&
> -					(ascq != 0x0e)) {
> +			if (scsi_sense_asc(sshdr) ==
> +			    ASC_TARGET_OPERATING_CONDITIONS_HAVE_CHANGED &&
> +			    sshdr->sense_code !=
> +			    INQUIRY_DATA_HAS_CHANGED &&
> +			    sshdr->sense_code !=
> +			    REPORTED_LUNS_DATA_HAS_CHANGED) {
>  				process_err_fn = storvsc_device_scan;
>  				set_host_byte(scmnd, DID_REQUEUE);
>  				goto do_work;

[Severity: High]
This is a pre-existing issue, but does this error handling path have a
use-after-free vulnerability during device teardown?

If the Hyper-V host sends a device removal request, storvsc_remove()
is called which destroys the workqueue before waiting for outstanding I/O
requests to drain:

storvsc_remove() {
    ...
    destroy_workqueue(host_dev->handle_error_wq);
    scsi_remove_host(host);
    storvsc_dev_remove(dev);
}

If the host then sends an I/O error completion (like a data overrun)
for an outstanding request while the host is being removed,
storvsc_command_completion() handles it by calling storvsc_handle_error().

When storvsc_handle_error() takes the goto do_work path shown above, it
will eventually call queue_work() using the already destroyed workqueue
pointer:

do_work:
    ...
    queue_work(host_dev->handle_error_wq, &wrk->work);

Can this sequence cause a use-after-free when queue_work accesses the freed
memory, and is this reachable by an untrusted Hyper-V host?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907024343.1046387-1-dlemoal@kernel.org?part=33