Re: [PATCH v2] scsi: storvsc: Support manual scans for all Hyper-V targets

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Kernel panic in fc_eh_timed_out() due to missing NULL check for fc_rport, contradicting the v2 changelog claim.
- [High] Use-After-Free race condition in fc_user_scan_tgt due to unpinned rport pointer.
--

--- Patch [1]: [PATCH v2] scsi: storvsc: Support manual scans for all Hyper-V targets ---
commit 7a84d34942989f4df1a7195141939c24f221c9ea
Author: Laurence Oberman <loberman@redhat.com>

scsi: storvsc: Support manual scans for all Hyper-V targets

This patch provides a storvsc-specific user_scan callback that uses the
exported scsi_scan_target() interface to allow explicit scanning of targets
without requiring synthetic fc_rports for each target.

> ---
> Changes in v2:
> - Replace the private, unexported scsi_scan_host_selected() helper with
>   the exported scsi_scan_target() interface.
> - Preserve wildcard channel and target scan behavior.
> - Verify that storvsc_eh_timed_out() does not use the FC error-handler
>   helpers; the reported missing-rport dereference is not present.

[Severity: High]
This isn't a bug introduced by this patch, but is the v2 changelog accurate
regarding storvsc_eh_timed_out()?

Looking at storvsc_eh_timed_out() in drivers/scsi/storvsc_drv.c, it appears
to explicitly route I/O timeouts to the FC transport error handler:

if (scmnd->device->host->transportt == fc_transport_template)
        return fc_eh_timed_out(scmnd);

When fc_eh_timed_out() handles this, it extracts the fc_rport pointer:

struct fc_rport *rport = starget_to_rport(scsi_target(scmd->device));
if (rport->port_state == FC_PORTSTATE_BLOCKED)

Because storvsc targets are created without an intermediate fc_rport parent,
starget_to_rport() returns NULL. Does this mean fc_eh_timed_out() will
immediately dereference a NULL pointer during an I/O timeout?


As a related observation on the FC transport layer scan paths:

[Severity: High]
This is a pre-existing issue, but does fc_user_scan_tgt() have a race
condition that could lead to a use-after-free?

In drivers/scsi/scsi_transport_fc.c:fc_user_scan_tgt(), the host lock is
dropped before scanning the target:

spin_unlock_irqrestore(shost->host_lock, flags);
scsi_scan_target(&rport->dev, channel, id, lun,
                 SCSI_SCAN_MANUAL);

If a concurrent FC link down event or port removal triggers
fc_rport_final_delete(), can the rport device be removed and freed before
or during the scsi_scan_target() execution? If so, should a reference be
acquired on rport->dev prior to dropping the lock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723163743.1274830-1-loberman@redhat.com?part=1