Re: [PATCH v2 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] The `sense_hdr` structure is read uninitialized in `storvsc_handle_error` if a command completes with `scsi_status == 0` but an SRB error status and the AUTOSENSE_VALID flag set.
--

commit 08f20503dd93893c68326306a042bfdb6e31394b
Author: Damien Le Moal <dlemoal@kernel.org>

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 defined in include/scsi/scsi_sense.h.

> 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

[ ... ]

> @@ -1141,8 +1145,7 @@ static void storvsc_command_completion(struct storvsc_cmd_request *cmd_request,
>  	}
>  
>  	if (vm_srb->srb_status != SRB_STATUS_SUCCESS) {
> -		storvsc_handle_error(vm_srb, scmnd, host, sense_hdr.asc,
> -					 sense_hdr.ascq);
> +		storvsc_handle_error(vm_srb, scmnd, host, &sense_hdr);

[Severity: High]
This isn't a bug introduced by this patch, but does passing &sense_hdr here
risk reading uninitialized memory if the command completes with
scsi_status == 0?

In storvsc_command_completion(), sense_hdr is declared on the stack without
initialization. It is only populated if scmnd->result is non-zero:

storvsc_command_completion() {
        struct scsi_sense_hdr sense_hdr;
        ...
        if (scmnd->result) {
                sense_ok = scsi_normalize_sense(..., &sense_hdr);
                ...
        }

        if (vm_srb->srb_status != SRB_STATUS_SUCCESS) {
                storvsc_handle_error(vm_srb, scmnd, host, &sense_hdr);

If the Hyper-V host returns scsi_status == 0 but an SRB error occurred
(for example, with AUTOSENSE_VALID set), storvsc_handle_error() is called
with the uninitialized sense_hdr.

Inside storvsc_handle_error(), this leads directly to evaluating uninitialized
stack memory when checking sshdr->sense_code for CAPACITY_DATA_HAS_CHANGED
or other statuses.

>  		/*
>  		 * The Windows driver set data_transfer_length on
>  		 * SRB_STATUS_DATA_OVERRUN. On other errors, this value

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