diff mbox series

usb-storage: Add Brain USB3-FW to IGNORE_UAS

Message ID 20240226075936.1744353-1-huxiaoying@kylinos.cn (mailing list archive)
State Superseded
Headers show
Series usb-storage: Add Brain USB3-FW to IGNORE_UAS | expand

Commit Message

Tom Hu Feb. 26, 2024, 7:59 a.m. UTC
The UAS mode of BRAIN USB_HDD is reported to fail to work on several
platforms with the following error message, then after re-connecting the
device will be offlined and not working at all.

[  622.518442][ 2] sd 8:0:0:0: [sda] tag#17 uas_eh_abort_handler 0 uas-tag 18
                   inflight: CMD
[  622.527575][ 2] sd 8:0:0:0: [sda] tag#17 CDB: Write(10) 2a 00 03 6f 88 00 00
                   04 00 00
[  622.536330][ 2] sd 8:0:0:0: [sda] tag#0 uas_eh_abort_handler 0 uas-tag 1
                   inflight: CMD
[  622.545266][ 2] sd 8:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 07 44 1a 88 00
                   00 08 00

These disks have a broken uas implementation, the tag field of the status
iu-s is not set properly, so we need to fall-back to usb-storage.

Signed-off-by: Hu Xiaoying <huxiaoying@kylinos.cn>
1
---
 drivers/usb/storage/unusual_uas.h | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Alan Stern March 1, 2024, 4:53 p.m. UTC | #1
On Mon, Feb 26, 2024 at 03:59:36PM +0800, Tom Hu wrote:
> The UAS mode of BRAIN USB_HDD is reported to fail to work on several
> platforms with the following error message, then after re-connecting the
> device will be offlined and not working at all.
> 
> [  622.518442][ 2] sd 8:0:0:0: [sda] tag#17 uas_eh_abort_handler 0 uas-tag 18
>                    inflight: CMD
> [  622.527575][ 2] sd 8:0:0:0: [sda] tag#17 CDB: Write(10) 2a 00 03 6f 88 00 00
>                    04 00 00
> [  622.536330][ 2] sd 8:0:0:0: [sda] tag#0 uas_eh_abort_handler 0 uas-tag 1
>                    inflight: CMD
> [  622.545266][ 2] sd 8:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 07 44 1a 88 00
>                    00 08 00
> 
> These disks have a broken uas implementation, the tag field of the status
> iu-s is not set properly, so we need to fall-back to usb-storage.
> 
> Signed-off-by: Hu Xiaoying <huxiaoying@kylinos.cn>
> 1

What is that 1?

> ---
>  drivers/usb/storage/unusual_uas.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h
> index 1f8c9b16a0fb..98b7ff2c76ba 100644
> --- a/drivers/usb/storage/unusual_uas.h
> +++ b/drivers/usb/storage/unusual_uas.h
> @@ -83,6 +83,13 @@ UNUSUAL_DEV(0x0bc2, 0x331a, 0x0000, 0x9999,
>  		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
>  		US_FL_NO_REPORT_LUNS),
>  
> +/* Reported-by: Tom Hu <huxiaoying@kylinos.cn> */
> +UNUSUAL_DEV(0x1234, 0x1234, 0x0000, 0x9999,

The vendor and product ID values have a suspicious look, but they appear 
to be genuine.

> +		"Brain",
> +		"External HDD",
> +		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
> +		US_FL_IGNORE_UAS),
> +
>  /* Reported-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> */
>  UNUSUAL_DEV(0x13fd, 0x3940, 0x0000, 0x9999,
>  		"Initio Corporation",
> -- 
> 2.25.1

Acked-by: Alan Stern <stern@rowland.harvard.edu>

Alan Stern
Lars Melin March 2, 2024, 12:06 p.m. UTC | #2
On 2024-03-01 23:53, Alan Stern wrote:
>>   drivers/usb/storage/unusual_uas.h | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h
>> index 1f8c9b16a0fb..98b7ff2c76ba 100644
>> --- a/drivers/usb/storage/unusual_uas.h
>> +++ b/drivers/usb/storage/unusual_uas.h
>> @@ -83,6 +83,13 @@ UNUSUAL_DEV(0x0bc2, 0x331a, 0x0000, 0x9999,
>>   		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
>>   		US_FL_NO_REPORT_LUNS),
>>   
>> +/* Reported-by: Tom Hu <huxiaoying@kylinos.cn> */
>> +UNUSUAL_DEV(0x1234, 0x1234, 0x0000, 0x9999,
> 
> The vendor and product ID values have a suspicious look, but they appear
> to be genuine.

Hi Alan,
it is of course a bogus Id, here is another one:

"SmartWi - Multi Room Solution is a Smart Card Reader hardware device. 
This driver was developed by SmartWi International A/S. The hardware id 
of this driver is USB/VID_1234&PID_1234."

found by googling vid_1234&pid_1234.

There are others like 1234:5678 which also looks suspicious.

thanks
Lars
Greg KH March 2, 2024, 7:19 p.m. UTC | #3
On Sat, Mar 02, 2024 at 07:06:20PM +0700, Lars Melin wrote:
> On 2024-03-01 23:53, Alan Stern wrote:
> > >   drivers/usb/storage/unusual_uas.h | 7 +++++++
> > >   1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h
> > > index 1f8c9b16a0fb..98b7ff2c76ba 100644
> > > --- a/drivers/usb/storage/unusual_uas.h
> > > +++ b/drivers/usb/storage/unusual_uas.h
> > > @@ -83,6 +83,13 @@ UNUSUAL_DEV(0x0bc2, 0x331a, 0x0000, 0x9999,
> > >   		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
> > >   		US_FL_NO_REPORT_LUNS),
> > > +/* Reported-by: Tom Hu <huxiaoying@kylinos.cn> */
> > > +UNUSUAL_DEV(0x1234, 0x1234, 0x0000, 0x9999,
> > 
> > The vendor and product ID values have a suspicious look, but they appear
> > to be genuine.
> 
> Hi Alan,
> it is of course a bogus Id, here is another one:
> 
> "SmartWi - Multi Room Solution is a Smart Card Reader hardware device. This
> driver was developed by SmartWi International A/S. The hardware id of this
> driver is USB/VID_1234&PID_1234."
> 
> found by googling vid_1234&pid_1234.
> 
> There are others like 1234:5678 which also looks suspicious.

Yeah, this is a "fake" number, which happens a lot (0000/0000 is also
used.)

Because of this, I don't know if we should take this change.  It
obviously fixes an issue for _this_ device, but for any other
usb-storage device using this fake id, will it cause issues?

thanks,

greg k-h
Lars Melin March 4, 2024, 2:29 a.m. UTC | #4
On 2024-03-03 02:19, Greg KH wrote:
> On Sat, Mar 02, 2024 at 07:06:20PM +0700, Lars Melin wrote:
>> On 2024-03-01 23:53, Alan Stern wrote:
>>>>    drivers/usb/storage/unusual_uas.h | 7 +++++++
>>>>    1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h
>>>> index 1f8c9b16a0fb..98b7ff2c76ba 100644
>>>> --- a/drivers/usb/storage/unusual_uas.h
>>>> +++ b/drivers/usb/storage/unusual_uas.h
>>>> @@ -83,6 +83,13 @@ UNUSUAL_DEV(0x0bc2, 0x331a, 0x0000, 0x9999,
>>>>    		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
>>>>    		US_FL_NO_REPORT_LUNS),
>>>> +/* Reported-by: Tom Hu <huxiaoying@kylinos.cn> */
>>>> +UNUSUAL_DEV(0x1234, 0x1234, 0x0000, 0x9999,
>>>
>>> The vendor and product ID values have a suspicious look, but they appear
>>> to be genuine.
>>
>> Hi Alan,
>> it is of course a bogus Id, here is another one:
>>
>> "SmartWi - Multi Room Solution is a Smart Card Reader hardware device. This
>> driver was developed by SmartWi International A/S. The hardware id of this
>> driver is USB/VID_1234&PID_1234."
>>
>> found by googling vid_1234&pid_1234.
>>
>> There are others like 1234:5678 which also looks suspicious.
> 
> Yeah, this is a "fake" number, which happens a lot (0000/0000 is also
> used.)
> 
> Because of this, I don't know if we should take this change.  It
> obviously fixes an issue for _this_ device, but for any other
> usb-storage device using this fake id, will it cause issues?
> 
> thanks,
> 
> greg k-h


If you are asking me then I say that the patch should not be taken.
If the manufacturer doesn't make it possible to distinguish this USB 
device from another USB device then there is nothing we can do for him.


thanks
Lars
diff mbox series

Patch

diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h
index 1f8c9b16a0fb..98b7ff2c76ba 100644
--- a/drivers/usb/storage/unusual_uas.h
+++ b/drivers/usb/storage/unusual_uas.h
@@ -83,6 +83,13 @@  UNUSUAL_DEV(0x0bc2, 0x331a, 0x0000, 0x9999,
 		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
 		US_FL_NO_REPORT_LUNS),
 
+/* Reported-by: Tom Hu <huxiaoying@kylinos.cn> */
+UNUSUAL_DEV(0x1234, 0x1234, 0x0000, 0x9999,
+		"Brain",
+		"External HDD",
+		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
+		US_FL_IGNORE_UAS),
+
 /* Reported-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> */
 UNUSUAL_DEV(0x13fd, 0x3940, 0x0000, 0x9999,
 		"Initio Corporation",