| Message ID | 20210601070439.1236679-1-yuyufen@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: libsas: check lun number valid for ata device in sas_queuecommand() | expand |
On 01/06/2021 08:04, Yufen Yu wrote: > We found that offline a ata device on hisi sas control and then > scanning the host can probe 255 not exist devices into system. > It can be reproduced easily as following: > > Some ata devices on hisi sas v3 control: > [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 not exist device 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:254] disk ATA WDC WD2003FYYS-3 1D01 /dev/sdja > [2:0:1:255] disk ATA WDC WD2003FYYS-3 1D01 /dev/sdjb > > When we try to scan the host, REPORT LUN command issued to the > offline device (sdb) will return fail. Then it tries to do a > sequential scan. Since only one ata device, any INQUIRY command > will be issued to the only device (i.e. lun 0) and return success, > no matter the lun number. So, the device whose lun number is not > zero will also be probed into system. > > We fix the probe by checking lun number valid in sas_queuecommand. > Any lun number is not equal '0' will return fail. > > Signed-off-by: Wu Bo <wubo40@huawei.com> > Signed-off-by: Yufen Yu <yuyufen@huawei.com> As mentioned privately, we could alternatively add a check in a slave alloc callback, like: int sas_slave_alloc(struct scsi_device *sdev) { if (dev_is_sata(sdev_to_domain_dev(sdev)) && sdev->lun) return -ENXIO; return 0; } This avoids touching the queuecommand fastpath. But not sure it is worth it. BTW, please mention that we are doing similar to commit 2fc62e2ac, to cut down the commit log a bit. > --- > drivers/scsi/libsas/sas_scsi_host.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c > index 1bf939818c98..62a01d11df96 100644 > --- a/drivers/scsi/libsas/sas_scsi_host.c > +++ b/drivers/scsi/libsas/sas_scsi_host.c > @@ -174,6 +174,12 @@ int sas_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) > } > > if (dev_is_sata(dev)) { > + /* sas ata just have one lun */ > + if (cmd->device->lun != 0) { > + cmd->result = (DID_BAD_TARGET << 16); > + cmd->scsi_done(cmd); > + return res; goto out_done is better > + } > spin_lock_irq(dev->sata_dev.ap->lock); > res = ata_sas_queuecmd(cmd, dev->sata_dev.ap); > spin_unlock_irq(dev->sata_dev.ap->lock); >
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c index 1bf939818c98..62a01d11df96 100644 --- a/drivers/scsi/libsas/sas_scsi_host.c +++ b/drivers/scsi/libsas/sas_scsi_host.c @@ -174,6 +174,12 @@ int sas_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) } if (dev_is_sata(dev)) { + /* sas ata just have one lun */ + if (cmd->device->lun != 0) { + cmd->result = (DID_BAD_TARGET << 16); + cmd->scsi_done(cmd); + return res; + } spin_lock_irq(dev->sata_dev.ap->lock); res = ata_sas_queuecmd(cmd, dev->sata_dev.ap); spin_unlock_irq(dev->sata_dev.ap->lock);