| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sun, 2 Feb 2020 11:47:49 -0500 (EST)
From: Alan Stern <stern@...land.harvard.edu>
To: Sven Schnelle <svens@...ckframe.org>
cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
<linux-usb@...r.kernel.org>,
<usb-storage@...ts.one-eyed-alien.net>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] usb-storage: Add support for Ratoc U2SCX multiple
device mode
On Sun, 2 Feb 2020, Sven Schnelle wrote:
> The Ratoc U2SCX can support more than one device but uses another
> USB ID in that case (0584:0222) with a Vendor specific class. The
> used protocol is still USB Mass storage, but we need to fetch the
> maximum LUN number as otherwise the controller would address all
> devices on the SCSI bus when an invalid LUN is probed. It looks like
> U2SCX maps SCSI ID to LUN numbers, so we must not enable
> US_FL_SCM_MULT_TARG.
>
> dmesg with the driver enabled looks like this:
>
> [ 133.157337] usb 1-2: new high-speed USB device number 6 using xhci_hcd
> [ 133.170273] usb 1-2: New USB device found, idVendor=0584, idProduct=0222, bcdDevice= 1.11
> [ 133.170279] usb 1-2: New USB device strings: Mfr=1, Product=3, SerialNumber=2
> [ 133.170283] usb 1-2: Product: USB-SCSI Converter
> [ 133.170286] usb 1-2: Manufacturer: RATOCSystems,Inc.
> [ 133.170289] usb 1-2: SerialNumber: 020301002197
> [ 133.171576] usb-storage 1-2:1.0: USB Mass Storage device detected
> [ 133.172084] scsi host1: usb-storage 1-2:1.0
> [ 134.199245] scsi 1:0:0:0: CD-ROM HL-DT-ST DVDRAM GSA-4163B A100 PQ: 0 ANSI: 2
> [ 134.202744] scsi 1:0:0:1: Direct-Access SyQuest SQ5200C A0L PQ: 0 ANSI: 2
> [ 134.219300] scsi 1:0:0:2: Direct-Access SyQuest SQ3270S 1_24 PQ: 0 ANSI: 2
> [ 134.224660] sr 1:0:0:0: Power-on or device reset occurred
> [ 134.343256] sr 1:0:0:0: [sr0] scsi3-mmc drive: 40x/40x writer dvd-ram cd/rw xa/form2 cdda tray
> [ 134.343262] cdrom: Uniform CD-ROM driver Revision: 3.20
> [ 134.350873] sr 1:0:0:0: Attached scsi CD-ROM sr0
> [ 134.351120] sr 1:0:0:0: Attached scsi generic sg1 type 5
> [ 134.351438] sd 1:0:0:1: Attached scsi generic sg2 type 0
> [ 134.351685] sd 1:0:0:2: Attached scsi generic sg3 type 0
> [ 134.362141] sd 1:0:0:1: Power-on or device reset occurred
> [ 134.483977] sd 1:0:0:1: [sdb] Attached SCSI removable disk
> [ 134.513922] sd 1:0:0:2: Power-on or device reset occurred
> [ 134.579802] sd 1:0:0:2: [sdc] Spinning up disk...
> [ 135.602397] ......
> [ 141.707017] sd 1:0:0:1: [sdb] Spinning up disk...
> [ 151.346332] .ready
> [ 151.353990] sd 1:0:0:2: [sdc] 524288 512-byte logical blocks: (268 MB/256 MiB)
> [ 151.361560] sd 1:0:0:2: [sdc] Write Protect is off
> [ 151.361566] sd 1:0:0:2: [sdc] Mode Sense: 99 00 00 08
> [ 151.368868] sd 1:0:0:2: [sdc] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA
> [ 151.428485] sdc: sdc1
> [ 151.465364] sd 1:0:0:2: [sdc] Attached SCSI removable disk
> [ 152.178231] .....ready
> [ 156.535115] sd 1:0:0:1: [sdb] 173456 512-byte logical blocks: (88.8 MB/84.7 MiB)
> [ 156.606445] sdb: sdb1
Isn't this the same as we would see for any multi-LUN device? What's
so special about this log that you thought including it here was
important?
> Signed-off-by: Sven Schnelle <svens@...ckframe.org>
> ---
> drivers/usb/storage/initializers.c | 27 +++++++++++++++++++++++++++
> drivers/usb/storage/initializers.h | 2 ++
> drivers/usb/storage/unusual_devs.h | 5 +++++
> 3 files changed, 34 insertions(+)
>
> diff --git a/drivers/usb/storage/initializers.c b/drivers/usb/storage/initializers.c
> index f8f9ce8dc710..089d67e99aa9 100644
> --- a/drivers/usb/storage/initializers.c
> +++ b/drivers/usb/storage/initializers.c
> @@ -44,6 +44,33 @@ int usb_stor_euscsi_init(struct us_data *us)
> return 0;
> }
>
> +/* Function to get the maximum LUN (Logical Unit number) from Ratoc. */
People reading this code almost certainly already know what "LUN"
stands for. You don't have to remind them.
> +int usb_stor_ratoc_u2scx_init(struct us_data *us)
> +{
> + int i, result;
> + uint8_t *idmap = (uint8_t *)us->iobuf;
> +
> + result = usb_stor_control_msg(us, us->recv_ctrl_pipe,
> + 0xe6, USB_DIR_IN | USB_TYPE_VENDOR |
> + USB_RECIP_INTERFACE,
> + 0x0, 0x0, idmap, 8, 5 * HZ);
> +
> + if (result < 0) {
> + usb_stor_dbg(us, "fetching max lun failed: %d\n", result);
> + return result;
> + }
You don't want to check that the device actually returned 8 bytes of
data? What if it returned fewer?
> +
> + for (i = 0; i < 7; i++) {
> + if (idmap[i] == 0xff)
You didn't initialize the contents of idmap. So if the device returned
fewer than 7 bytes, what makes you think the extra value will be 0xff?
> + break;
> + }
> +
> + if (i == 0)
> + return -EIO;
> +
> + us->max_lun = i - 1;
> + return 0;
> +}
> /*
> * This function is required to activate all four slots on the UCR-61S2B
> * flash reader
> diff --git a/drivers/usb/storage/initializers.h b/drivers/usb/storage/initializers.h
> index 2dbf9c7d9749..b3ad7bfadf41 100644
> --- a/drivers/usb/storage/initializers.h
> +++ b/drivers/usb/storage/initializers.h
> @@ -37,3 +37,5 @@ int usb_stor_ucr61s2b_init(struct us_data *us);
>
> /* This places the HUAWEI E220 devices in multi-port mode */
> int usb_stor_huawei_e220_init(struct us_data *us);
> +
> +int usb_stor_ratoc_u2scx_init(struct us_data *us);
> diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h
> index 1cd9b6305b06..46b95944533c 100644
> --- a/drivers/usb/storage/unusual_devs.h
> +++ b/drivers/usb/storage/unusual_devs.h
> @@ -2372,6 +2372,11 @@ UNUSUAL_DEV( 0xed10, 0x7636, 0x0001, 0x0001,
> "Digital MP3 Audio Player",
> USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NOT_LOCKABLE ),
>
> +UNUSUAL_DEV(0x0584, 0x0222, 0x0000, 0xffff,
> + "Ratoc",
> + "U2SCX USB SCSI converter",
> + USB_SC_DEVICE, USB_PR_DEVICE, usb_stor_ratoc_u2scx_init, 0),
> +
Please read the comments at the start of the file, in particular the
part describing how the entries should e sorted.
Alan Stern
> /* Unusual uas devices */
> #if IS_ENABLED(CONFIG_USB_UAS)
> #include "unusual_uas.h"
>
Powered by blists - more mailing lists