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? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
