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

Pre-existing issues:
- [High] If storvsc_do_io() returns an error other than -EAGAIN, 
storvsc_queuecommand() incorrectly returns 0 to the SCSI midlayer, leaking the 
dynamically allocated payload and causing a command timeout/hang.
--

--- Patch [1]: [PATCH V2 1/1] x86/VMBus: Confidential VMBus for dynamic DMA 
transfers ---
commit 0fcd974d529c3a7f6e32e98d08ecd19b7a1e7b0a
Author: Tianyu Lan <[email protected]>

x86/VMBus: Confidential VMBus for dynamic DMA transfers
    
This commit modifies the VMBus driver to support dynamic DMA transfers
for Confidential VMs. It introduces the co_external_memory flag to bypass
the swiotlb and DMA API, allowing direct DMA to private encrypted memory.

> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 571ea549152b1..656fe73e0d35e 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c

[ ... ]

> @@ -1870,7 +1882,7 @@ static enum scsi_qc_status storvsc_queuecommand(struct 
> Scsi_Host *host,
>       ret = storvsc_do_io(dev, cmd_request, smp_processor_id());
>       migrate_enable();
>  
> -     if (ret)
> +     if (ret && (!dev->channel->co_external_memory))
>               scsi_dma_unmap(scmnd);
>  
>       if (ret == -EAGAIN) {

[Severity: High]
This is a pre-existing issue, but if storvsc_do_io() returns an error
other than -EAGAIN, does storvsc_queuecommand() leak the payload buffer
and silently drop the error?

If storvsc_do_io() returns a failure like -ENODEV, the code bypasses the
err_free_payload block and returns 0 to the SCSI midlayer:

storvsc_queuecommand() {
    ...
    ret = storvsc_do_io(dev, cmd_request, smp_processor_id());
    ...
    if (ret == -EAGAIN) {
        ret = SCSI_MLQUEUE_DEVICE_BUSY;
        goto err_free_payload;
    }

    return 0;

err_free_payload:
    if (payload_sz > sizeof(cmd_request->mpb))
        kfree(payload);

    return ret;
}

Since 0 is returned, the midlayer believes the command queued successfully,
but because it was never sent to the host, scsi_done() is never called.
Will this cause the command to linger until a SCSI command timeout occurs
while also permanently leaking the dynamically allocated payload?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to