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: > > 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 <[email protected]> > > 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?
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
