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

On Thu, 2026-07-23 at 16:51 +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider:
>=20
> 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.
> --
>=20
> --- Patch [1]: [PATCH v2] scsi: storvsc: Support manual scans for all
> Hyper-V targets ---
> commit 7a84d34942989f4df1a7195141939c24f221c9ea
> Author: Laurence Oberman <[email protected]>
>=20
> scsi: storvsc: Support manual scans for all Hyper-V targets
>=20
> 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.
>=20
> > ---
> > Changes in v2:
> > - Replace the private, unexported scsi_scan_host_selected() helper
> > with
> > =C2=A0 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
> > =C2=A0 helpers; the reported missing-rport dereference is not present.
>=20
> [Severity: High]
> This isn't a bug introduced by this patch, but is the v2 changelog
> accurate
> regarding storvsc_eh_timed_out()?
>=20
> 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:
>=20
> if (scmnd->device->host->transportt =3D=3D fc_transport_template)
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return fc_eh_timed_out(scmnd);
>=20
> When fc_eh_timed_out() handles this, it extracts the fc_rport
> pointer:
>=20
> struct fc_rport *rport =3D starget_to_rport(scsi_target(scmd->device));
> if (rport->port_state =3D=3D FC_PORTSTATE_BLOCKED)
>=20
> 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?
>=20
>=20
> As a related observation on the FC transport layer scan paths:
>=20
> [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?
>=20
> In drivers/scsi/scsi_transport_fc.c:fc_user_scan_tgt(), the host lock
> is
> dropped before scanning the target:
>=20
> spin_unlock_irqrestore(shost->host_lock, flags);
> scsi_scan_target(&rport->dev, channel, id, lun,
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0 SCSI_SCAN_MANUAL);
>=20
> 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?

I do not see any of F/C related calls in what I modified or in the code
I worked on so for me v2 is the correct and final version of this.

Thanks
Laurence