Linux SCSI subsystem development
 help / color / mirror / Atom feed
* [PATCH] scsi: storvsc: Support manual scans for all Hyper-V targets
@ 2026-07-23 15:14 Laurence Oberman
  2026-07-23 15:30 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: Laurence Oberman @ 2026-07-23 15:14 UTC (permalink / raw)
  To: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li
  Cc: James E.J. Bottomley, Martin K. Petersen, Laurence Oberman,
	linux-hyperv, linux-scsi, linux-kernel

The Fibre Channel transport topology created by storvsc exposes only one
dummy remote port per SCSI host. Its scsi_target_id is always zero.

As a result, the FC transport user-scan path performs its remote-port lookup
using target ID 0 and cannot initiate a scan for Target 1 or higher. No SCSI
command is therefore sent to Hyper-V when userspace explicitly requests a
scan of one of these targets.

storvsc itself supports up to STORVSC_FC_MAX_TARGETS and already passes
scmnd->device->id to Hyper-V as vm_srb->target_id. Devices on Target 1 and
higher work when initially discovered and can be rediscovered by a full host
scan, such as the scan triggered after an FC port bounce.

Provide a storvsc-specific user_scan callback that directly invokes
scsi_scan_host_selected(). This bypasses the single-rport lookup in
fc_user_scan() and allows userspace to explicitly scan any target supported
by storvsc.

This avoids requiring storvsc to create and manage one synthetic fc_rport
for every Hyper-V target.

Signed-off-by: Laurence Oberman <loberman@redhat.com>
---
 drivers/scsi/storvsc_drv.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 571ea549152b..80830aa12532 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -33,6 +33,7 @@
 #include <scsi/scsi_dbg.h>
 #include <scsi/scsi_transport_fc.h>
 #include <scsi/scsi_transport.h>
+#include "scsi_priv.h"
 
 /*
  * All wire protocol details (storage protocol between the guest and the host)
@@ -520,6 +521,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);
+}
+#endif
+
 static void storvsc_remove_lun(struct work_struct *work)
 {
 	struct storvsc_scan_work *wrk;
@@ -2232,6 +2244,8 @@ static int __init storvsc_drv_init(void)
 	fc_transport_template = fc_attach_transport(&fc_transport_functions);
 	if (!fc_transport_template)
 		return -ENODEV;
+
+	fc_transport_template->user_scan = storvsc_user_scan;
 #endif
 
 	ret = vmbus_driver_register(&storvsc_drv);
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] scsi: storvsc: Support manual scans for all Hyper-V targets
  2026-07-23 15:14 [PATCH] scsi: storvsc: Support manual scans for all Hyper-V targets Laurence Oberman
@ 2026-07-23 15:30 ` sashiko-bot
  2026-07-23 15:43   ` Laurence Oberman
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-07-23 15:30 UTC (permalink / raw)
  To: Laurence Oberman; +Cc: linux-scsi, linux-hyperv

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] scsi: storvsc: Support manual scans for all Hyper-V targets
  2026-07-23 15:30 ` sashiko-bot
@ 2026-07-23 15:43   ` Laurence Oberman
  0 siblings, 0 replies; 3+ messages in thread
From: Laurence Oberman @ 2026-07-23 15:43 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-scsi, linux-hyperv

On Thu, 2026-07-23 at 15:30 +0000, sashiko-bot@kernel.org wrote:
> 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
> > +
> 
> [ ... ]
Thanks for the review.

The module-build observation is correct. scsi_scan_host_selected() is
an internal SCSI midlayer helper and is not exported for use by a
modular storvsc driver. The successful built-in kernel build did not
expose that problem so I never caught it.

The callback should instead use the exported scsi_scan_target()
interface and should not include scsi_priv.h.

I am also reviewing the FC error-handler concern before posting a
revised patch. Devices discovered directly below the SCSI host do not
have an fc_rport parent, while the storvsc timeout path delegates to
fc_eh_timed_out(). A revised patch must ensure that this topology
cannot lead to a NULL rport dereference.

I will address both points in v2.

Thanks,
Laurence


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-23 15:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-23 15:14 [PATCH] scsi: storvsc: Support manual scans for all Hyper-V targets Laurence Oberman
2026-07-23 15:30 ` sashiko-bot
2026-07-23 15:43   ` Laurence Oberman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox