On 5/23/23 21:29, Jason Yan wrote: > On 2023/5/23 19:52, Damien Le Moal wrote: >>> I wonder if you can change the type of devno to 'unsigned int'? At a >>> closer look I found the user can control this value and may pass in a >>> bogus channel or id. >>> >>> proc_scsi_write >>> =>scsi_add_single_device >>> =>ata_scsi_user_scan >>> =>ata_find_dev >> Reading more about scsi_add_single_device(), the comment says "Note: >> this seems >> to be aimed exclusively at SCSI parallel busses.". So I don't think we >> should >> worry about it. But then I also do not understand why libata is wired >> to this at >> all. Cannot have ATA device on a parallel SCSI bus... > > The comment is kind of obsolete. It is not limited to SCSI parallel > busses only. > >> >> On my system, I cannot get >> >> echo "scsi add-single-device X 0 100 0" >/proc/scsi/scsi >> >> to do anything and so I do not see how ata_scsi_user_scan can ever be >> called... >> > > Did you enabled CONFIG_SCSI_PROC_FS ? I started a qemu and it still works. > > [root@localhost ~]# cat /proc/scsi/scsi > Attached devices: > Host: scsi1 Channel: 00 Id: 00 Lun: 00 > Vendor: QEMU Model: QEMU DVD-ROM Rev: 2.5+ > Type: CD-ROM ANSI SCSI revision: 05 > [root@localhost ~]# lsscsi > [1:0:0:0] cd/dvd QEMU QEMU DVD-ROM 2.5+ /dev/sr0 > [root@localhost ~]# > [root@localhost ~]# > [root@localhost ~]# echo "scsi remove-single-device 1 0 0 0" > > /proc/scsi/scsi > [ 639.747836] ata2.00: disable device > [root@localhost ~]# > [root@localhost ~]# > [root@localhost ~]# echo "scsi add-single-device 1 0 0 0" > /proc/scsi/scsi > [root@localhost ~]# > [root@localhost ~]# > [root@localhost ~]# lsscsi > [1:0:0:0] cd/dvd QEMU QEMU DVD-ROM 2.5+ /dev/sr0 > [root@localhost ~]# > [root@localhost ~]# > [root@localhost ~]# echo "scsi add-single-device 1 0 100 0" > > /proc/scsi/scsi > -bash: echo: write error: Invalid argument > [root@localhost ~]# > [root@localhost ~]# > > > For a wrong scsi nubmer "1 0 100 0", it returns an error now. If our > patch is applied, it will return ok and will add "1 0 0 0" instead, I > guess. Indeed. Using ata_find_dev() in ata_scsi_user_scan() as it is is not broken. And ata_scsi_user_scan() is also likely broken in subtle ways for libsas due to the ata port being determined from the scsi host, which does not seem to be how libsas manages things. Need to dig further. > > > Thanks, > Jason -- Damien Le Moal Western Digital Research
