[PATCH v3] scsi: libsas: add lun number check in .slave_alloc callback

[PATCH v3] scsi: libsas: add lun number check in .slave_alloc callback

[Yufen Yu]

We found that offline a sata device on hisi sas control and then
scanning the host can probe 255 not-existant devices into system.

[root@localhost ~]# lsscsi
  [2:0:0:0]    disk    ATA      Samsung SSD 860  2B6Q  /dev/sda
  [2:0:1:0]    disk    ATA      WDC WD2003FYYS-3 1D01  /dev/sdb
  [2:0:2:0]    disk    SEAGATE  ST600MM0006      B001  /dev/sdc

 1) echo "offline" > /sys/block/sdb/device/state
 2) echo "- - -" > /sys/class/scsi_host/host2/scan

Then, we can see another 255 non-existant devices in system:
  [root@localhost ~]# lsscsi
  [2:0:0:0]    disk    ATA      Samsung SSD 860  2B6Q  /dev/sda
  [2:0:1:0]    disk    ATA      WDC WD2003FYYS-3 1D01  /dev/sdb
  [2:0:1:1]    disk    ATA      WDC WD2003FYYS-3 1D01  /dev/sdh
  ...
  [2:0:1:255]  disk    ATA      WDC WD2003FYYS-3 1D01  /dev/sdjb

After REPORT LUN command issued to the offline device fail, it tries
to do a sequential scan and probe all devices whose lun is not 0
successfully.

To fix the problem, we try to do same things as commit 2fc62e2ac350
("[SCSI] libsas: disable scanning lun > 0 on ata devices"), which
will prevent the device whose lun number is not zero probe into system.

Reported-by: Wu Bo <wubo40@huawei.com>
Suggested-by: John Garry <john.garry@huawei.com>
Signed-off-by: Yufen Yu <yuyufen@huawei.com>
---
 drivers/scsi/aic94xx/aic94xx_init.c    | 1 +
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 1 +
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 1 +
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 1 +
 drivers/scsi/isci/init.c               | 1 +
 drivers/scsi/libsas/sas_scsi_host.c    | 9 +++++++++
 drivers/scsi/mvsas/mv_init.c           | 1 +
 drivers/scsi/pm8001/pm8001_init.c      | 1 +
 8 files changed, 16 insertions(+)

v2->v3:
    simplify commit log and fix some spelling error

diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c
index a195bfe9eccc..7a78606598c4 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -53,6 +53,7 @@ static struct scsi_host_template aic94xx_sht = {
 	.max_sectors		= SCSI_DEFAULT_MAX_SECTORS,
 	.eh_device_reset_handler	= sas_eh_device_reset_handler,
 	.eh_target_reset_handler	= sas_eh_target_reset_handler,
+	.slave_alloc		= sas_slave_alloc,
 	.target_destroy		= sas_target_destroy,
 	.ioctl			= sas_ioctl,
 #ifdef CONFIG_COMPAT
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index 3e359ac752fd..15eaac3a4eb6 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -1771,6 +1771,7 @@ static struct scsi_host_template sht_v1_hw = {
 	.max_sectors		= SCSI_DEFAULT_MAX_SECTORS,
 	.eh_device_reset_handler = sas_eh_device_reset_handler,
 	.eh_target_reset_handler = sas_eh_target_reset_handler,
+	.slave_alloc		= sas_slave_alloc,
 	.target_destroy		= sas_target_destroy,
 	.ioctl			= sas_ioctl,
 #ifdef CONFIG_COMPAT
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 46f60fc2a069..9df1639ffa65 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -3584,6 +3584,7 @@ static struct scsi_host_template sht_v2_hw = {
 	.max_sectors		= SCSI_DEFAULT_MAX_SECTORS,
 	.eh_device_reset_handler = sas_eh_device_reset_handler,
 	.eh_target_reset_handler = sas_eh_target_reset_handler,
+	.slave_alloc		= sas_slave_alloc,
 	.target_destroy		= sas_target_destroy,
 	.ioctl			= sas_ioctl,
 #ifdef CONFIG_COMPAT
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index e95408314078..36ec3901cfd4 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -3155,6 +3155,7 @@ static struct scsi_host_template sht_v3_hw = {
 	.max_sectors		= SCSI_DEFAULT_MAX_SECTORS,
 	.eh_device_reset_handler = sas_eh_device_reset_handler,
 	.eh_target_reset_handler = sas_eh_target_reset_handler,
+	.slave_alloc		= sas_slave_alloc,
 	.target_destroy		= sas_target_destroy,
 	.ioctl			= sas_ioctl,
 #ifdef CONFIG_COMPAT
diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
index c452849e7bb4..ffd33e5decae 100644
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -167,6 +167,7 @@ static struct scsi_host_template isci_sht = {
 	.eh_abort_handler		= sas_eh_abort_handler,
 	.eh_device_reset_handler        = sas_eh_device_reset_handler,
 	.eh_target_reset_handler        = sas_eh_target_reset_handler,
+	.slave_alloc			= sas_slave_alloc,
 	.target_destroy			= sas_target_destroy,
 	.ioctl				= sas_ioctl,
 #ifdef CONFIG_COMPAT
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 1bf939818c98..ee44a0d7730b 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -911,6 +911,14 @@ void sas_task_abort(struct sas_task *task)
 		blk_abort_request(sc->request);
 }
 
+int sas_slave_alloc(struct scsi_device *sdev)
+{
+	if (dev_is_sata(sdev_to_domain_dev(sdev)) && sdev->lun)
+		return -ENXIO;
+
+	return 0;
+}
+
 void sas_target_destroy(struct scsi_target *starget)
 {
 	struct domain_device *found_dev = starget->hostdata;
@@ -957,5 +965,6 @@ EXPORT_SYMBOL_GPL(sas_task_abort);
 EXPORT_SYMBOL_GPL(sas_phy_reset);
 EXPORT_SYMBOL_GPL(sas_eh_device_reset_handler);
 EXPORT_SYMBOL_GPL(sas_eh_target_reset_handler);
+EXPORT_SYMBOL_GPL(sas_slave_alloc);
 EXPORT_SYMBOL_GPL(sas_target_destroy);
 EXPORT_SYMBOL_GPL(sas_ioctl);
diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
index 6aa2697c4a15..b03c0f35d7b0 100644
--- a/drivers/scsi/mvsas/mv_init.c
+++ b/drivers/scsi/mvsas/mv_init.c
@@ -46,6 +46,7 @@ static struct scsi_host_template mvs_sht = {
 	.max_sectors		= SCSI_DEFAULT_MAX_SECTORS,
 	.eh_device_reset_handler = sas_eh_device_reset_handler,
 	.eh_target_reset_handler = sas_eh_target_reset_handler,
+	.slave_alloc		= sas_slave_alloc,
 	.target_destroy		= sas_target_destroy,
 	.ioctl			= sas_ioctl,
 #ifdef CONFIG_COMPAT
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index af09bd282cb9..313248c7bab9 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -101,6 +101,7 @@ static struct scsi_host_template pm8001_sht = {
 	.max_sectors		= SCSI_DEFAULT_MAX_SECTORS,
 	.eh_device_reset_handler = sas_eh_device_reset_handler,
 	.eh_target_reset_handler = sas_eh_target_reset_handler,
+	.slave_alloc		= sas_slave_alloc,
 	.target_destroy		= sas_target_destroy,
 	.ioctl			= sas_ioctl,
 #ifdef CONFIG_COMPAT
-- 
2.25.4

Re: [PATCH v3] scsi: libsas: add lun number check in .slave_alloc callback

[John Garry]

On 22/06/2021 04:40, Yufen Yu wrote:
> We found that offline a sata device on hisi sas control and then

/s/sata/SATA/

/s/control/controller/

> scanning the host can probe 255 not-existant devices into system.
> 
> [root@localhost ~]# lsscsi
>    [2:0:0:0]    disk    ATA      Samsung SSD 860  2B6Q  /dev/sda
>    [2:0:1:0]    disk    ATA      WDC WD2003FYYS-3 1D01  /dev/sdb
>    [2:0:2:0]    disk    SEAGATE  ST600MM0006      B001  /dev/sdc
> 
>   1) echo "offline" > /sys/block/sdb/device/state
>   2) echo "- - -" > /sys/class/scsi_host/host2/scan
> 
> Then, we can see another 255 non-existant devices in system:
>    [root@localhost ~]# lsscsi
>    [2:0:0:0]    disk    ATA      Samsung SSD 860  2B6Q  /dev/sda
>    [2:0:1:0]    disk    ATA      WDC WD2003FYYS-3 1D01  /dev/sdb
>    [2:0:1:1]    disk    ATA      WDC WD2003FYYS-3 1D01  /dev/sdh
>    ...
>    [2:0:1:255]  disk    ATA      WDC WD2003FYYS-3 1D01  /dev/sdjb
> 
> After REPORT LUN command issued to the offline device fail, it tries
> to do a sequential scan and probe all devices whose lun is not 0
> successfully.
> 
> To fix the problem, we try to do same things as commit 2fc62e2ac350
> ("[SCSI] libsas: disable scanning lun > 0 on ata devices"), which
> will prevent the device whose lun number is not zero probe into system.
> 

I thought that we would mention why we don't change aic7xxx driver.

> Reported-by: Wu Bo<wubo40@huawei.com>
> Suggested-by: John Garry<john.garry@huawei.com>
> Signed-off-by: Yufen Yu<yuyufen@huawei.com>
> ---

Apart from the small items above:
Reviewed-by: John Garry <john.garry@huawei.com>

Re: [PATCH v3] scsi: libsas: add lun number check in .slave_alloc callback

[Jason Yan]

在 2021/6/22 15:20, John Garry 写道:
> On 22/06/2021 04:40, Yufen Yu wrote:
>> We found that offline a sata device on hisi sas control and then
> 
> /s/sata/SATA/
> 
> /s/control/controller/
> 
>> scanning the host can probe 255 not-existant devices into system.
>>
>> [root@localhost ~]# lsscsi
>>    [2:0:0:0]    disk    ATA      Samsung SSD 860  2B6Q  /dev/sda
>>    [2:0:1:0]    disk    ATA      WDC WD2003FYYS-3 1D01  /dev/sdb
>>    [2:0:2:0]    disk    SEAGATE  ST600MM0006      B001  /dev/sdc
>>
>>   1) echo "offline" > /sys/block/sdb/device/state
>>   2) echo "- - -" > /sys/class/scsi_host/host2/scan
>>
>> Then, we can see another 255 non-existant devices in system:
>>    [root@localhost ~]# lsscsi
>>    [2:0:0:0]    disk    ATA      Samsung SSD 860  2B6Q  /dev/sda
>>    [2:0:1:0]    disk    ATA      WDC WD2003FYYS-3 1D01  /dev/sdb
>>    [2:0:1:1]    disk    ATA      WDC WD2003FYYS-3 1D01  /dev/sdh
>>    ...
>>    [2:0:1:255]  disk    ATA      WDC WD2003FYYS-3 1D01  /dev/sdjb
>>
>> After REPORT LUN command issued to the offline device fail, it tries
>> to do a sequential scan and probe all devices whose lun is not 0
>> successfully.
>>
>> To fix the problem, we try to do same things as commit 2fc62e2ac350
>> ("[SCSI] libsas: disable scanning lun > 0 on ata devices"), which
>> will prevent the device whose lun number is not zero probe into system.
>>
> 
> I thought that we would mention why we don't change aic7xxx driver.

We are talking about libsas here. aic7xxx is not even a libsas driver,
so I don't think we need to mention it.

Thanks,
Jason

> 
>> Reported-by: Wu Bo<wubo40@huawei.com>
>> Suggested-by: John Garry<john.garry@huawei.com>
>> Signed-off-by: Yufen Yu<yuyufen@huawei.com>
>> ---
> 
> Apart from the small items above:
> Reviewed-by: John Garry <john.garry@huawei.com>
> .

Re: [PATCH v3] scsi: libsas: add lun number check in .slave_alloc callback

[Jason Yan]

在 2021/6/22 11:40, Yufen Yu 写道:
> We found that offline a sata device on hisi sas control and then
> scanning the host can probe 255 not-existant devices into system.
> 
> [root@localhost ~]# lsscsi
>    [2:0:0:0]    disk    ATA      Samsung SSD 860  2B6Q  /dev/sda
>    [2:0:1:0]    disk    ATA      WDC WD2003FYYS-3 1D01  /dev/sdb
>    [2:0:2:0]    disk    SEAGATE  ST600MM0006      B001  /dev/sdc
> 
>   1) echo "offline" > /sys/block/sdb/device/state
>   2) echo "- - -" > /sys/class/scsi_host/host2/scan
> 
> Then, we can see another 255 non-existant devices in system:
>    [root@localhost ~]# lsscsi
>    [2:0:0:0]    disk    ATA      Samsung SSD 860  2B6Q  /dev/sda
>    [2:0:1:0]    disk    ATA      WDC WD2003FYYS-3 1D01  /dev/sdb
>    [2:0:1:1]    disk    ATA      WDC WD2003FYYS-3 1D01  /dev/sdh
>    ...
>    [2:0:1:255]  disk    ATA      WDC WD2003FYYS-3 1D01  /dev/sdjb
> 
> After REPORT LUN command issued to the offline device fail, it tries
> to do a sequential scan and probe all devices whose lun is not 0
> successfully.
> 
> To fix the problem, we try to do same things as commit 2fc62e2ac350
> ("[SCSI] libsas: disable scanning lun > 0 on ata devices"), which
> will prevent the device whose lun number is not zero probe into system.
> 
> Reported-by: Wu Bo <wubo40@huawei.com>
> Suggested-by: John Garry <john.garry@huawei.com>
> Signed-off-by: Yufen Yu <yuyufen@huawei.com>
> ---

Reviewed-by: Jason Yan <yanaijie@huawei.com>

Re: [PATCH v3] scsi: libsas: add lun number check in .slave_alloc callback

[John Garry]

On 22/06/2021 10:15, Jason Yan wrote:
>>
>> I thought that we would mention why we don't change aic7xxx driver.
> 
> We are talking about libsas here. aic7xxx is not even a libsas driver,
> so I don't think we need to mention it.

Ah, right. I don't know why it was mentioned when we were talking about 
the v2 of this patch.

Thanks,
John

Re: [PATCH v3] scsi: libsas: add lun number check in .slave_alloc callback

[Martin K. Petersen]

Yufen,

> We found that offline a sata device on hisi sas control and then
> scanning the host can probe 255 not-existant devices into system.

Applied to 5.14/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v3] scsi: libsas: add lun number check in .slave_alloc callback

[Martin K. Petersen]

On Tue, 22 Jun 2021 11:40:37 +0800, Yufen Yu wrote:

> We found that offline a sata device on hisi sas control and then
> scanning the host can probe 255 not-existant devices into system.
> 
> [root@localhost ~]# lsscsi
>   [2:0:0:0]    disk    ATA      Samsung SSD 860  2B6Q  /dev/sda
>   [2:0:1:0]    disk    ATA      WDC WD2003FYYS-3 1D01  /dev/sdb
>   [2:0:2:0]    disk    SEAGATE  ST600MM0006      B001  /dev/sdc
> 
> [...]

Applied to 5.14/scsi-queue, thanks!

[1/1] scsi: libsas: add lun number check in .slave_alloc callback
      https://git.kernel.org/mkp/scsi/c/49da96d77938

-- 
Martin K. Petersen	Oracle Linux Engineering