Re: [PATCH] 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:

New issues:
- [Low] The patch calls the unexported internal midlayer function `scsi_scan_host_selected()` and includes the private header `"scsi_priv.h"` from a hardware driver, which will break module builds.

Pre-existing issues:
- [High] Timeouts on targets scanned without an `fc_rport` parent will cause a NULL pointer dereference in the FC transport error handler.
--

--- Patch [1]: [PATCH] scsi: storvsc: Support manual scans for all Hyper-V targets ---
commit ce267d7e85b7a51757a8afe1188939d3c5146d27
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 directly
invokes scsi_scan_host_selected(). This bypasses the single-rport lookup
in fc_user_scan() to allow userspace to explicitly scan any target
supported by storvsc. The goal is to avoid requiring storvsc to create
and manage synthetic fc_rports for every Hyper-V target.

> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index f03dc03a42c35..eb40473a8aad6 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -43,6 +43,7 @@
>  #include <scsi/scsi_dbg.h>
>  #include <scsi/scsi_transport_fc.h>
>  #include <scsi/scsi_transport.h>
> +#include "scsi_priv.h"

[Severity: Low]
Will this cause an architectural layering violation? Including a private
midlayer header like scsi_priv.h from a low-level hardware driver normally
breaks subsystem encapsulation.

[ ... ]

> @@ -550,6 +551,17 @@ static void storvsc_host_scan(struct work_struct *work)
>  	scsi_scan_host(host);
>  }
>  
> +#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
> +static int storvsc_user_scan(struct Scsi_Host *host,
> +			     unsigned int channel,
> +			     unsigned int id,
> +			     u64 lun)
> +{
> +	return scsi_scan_host_selected(host, channel, id, lun,
> +				       SCSI_SCAN_MANUAL);
> +}

[Severity: Low]
Does this cause a module build failure during the modpost link stage?

The function scsi_scan_host_selected() in drivers/scsi/scsi_scan.c lacks an
EXPORT_SYMBOL directive. If the kernel is configured to build storvsc as a
module (CONFIG_HYPERV_STORAGE=m), it seems it will fail to link.

[Severity: High]
This is a pre-existing issue, but does creating a topology without an
fc_rport parent lead to a NULL pointer dereference during error handling?

If a SCSI command times out on a target scanned directly under the SCSI host
(which this manual scan facilitates), storvsc_eh_timed_out() delegates to
the FC transport's fc_eh_timed_out(). Because the target was scanned under
the host device instead of an fc_rport, starget_to_rport() evaluates to NULL.

This NULL pointer is then unconditionally dereferenced in
drivers/scsi/scsi_transport_fc.c:fc_eh_timed_out():

fc_eh_timed_out()
{
    ...
    struct fc_rport *rport = starget_to_rport(scsi_target(scmd->device));

    if (rport->port_state == FC_PORTSTATE_BLOCKED)
    ...
}

Can timeouts under hardware or network load on these targets trigger a
kernel panic?

> +#endif
> +

[ ... ]

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