[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <34aa2eea-5417-4e46-91a0-aac1a46a84cf@gmail.com>
Date: Sun, 27 Aug 2023 18:45:55 +0200
From: Milan Broz <gmazyland@...il.com>
To: Alan Stern <stern@...land.harvard.edu>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-usb@...r.kernel.org, usb-storage@...ts.one-eyed-alien.net,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: usb-storage: how to extend quirks flags to 64bit?
On 8/27/23 17:50, Alan Stern wrote:
> On Sun, Aug 27, 2023 at 11:32:05AM +0200, Milan Broz wrote:
>> Hello,
>>
>> I tried to extend USB storage for the passthrough of Opal
>> security commands,
>
> What sort of changes are needed? Where is this passthrough mechanism
> documented?
We are currently adding support for optional OPAL hw encryption to
cryptsetup/LUKS2 (that uses kernel OPAL ioctl interface) and I tried
to make USB adapters to work with it too.
I'll send RFC patchset (it is quite simple) where I explain it in detail.
The patch for USB storage is actually one liner, the rest is in SCSI driver :)
Basically, USB adapters (not supporting UAS) cannot work as
required SCSI SECURITY IN/OUT SCSI commands do not work here.
But we can use ATA12 pass-thru (as used with original sedutils
and some other tools we used in research; it is a documented feature).
It works once ATA12 wrapper is added to block layer and USB storage enables
the "security_supported" bit.
>
>> and some adapters are clearly "not perfect".
>
> Which ones?
Namely Realtek 9210 family (NVME to USB bridge). Everything OPAL related
works, but the adapter always set write-protected bit for the whole
drive (even if OPAL locking range is just covering part of the disk).
I spent quite a lot time trying new firmware versions - this issue is
still there.
On the other side, many other USB to SATA bridges works nicely.
I think this is the exact situation where we should set a new quirks flag
to disable it. (The nasty thing is that for unbricking it you need PSID reset
- PSID is a number written on the label of the drive - followed by physical
disconnect for recovery.)
Anyway, I intentionally sent this 32bit flags question separately as it
is actually a generic issue - we are just out of flag space now...
Even if the patches mentioned above are rejected, someone will need
a new flag for something else later.
>> I would need to introduce a new quirks flag to turn it off.
>>
>> Seems that we are already out of quirks flags on 32bit
>> for usb storage - in usb_usual.h the last entry in mainline is
>> US_FLAG(SENSE_AFTER_SYNC, 0x80000000)
>>
>> Adding a new flag will work for 64-bit systems but not
>> for platforms with 32-bit unsigned long like i686.
>>
>> How do we allow new flag definitions?
>>
>> Struct us_data fflags can be made 64bit (defined in
>> drivers/usb/storage/usb.h), but the major problem is that these
>> are transferred through the generic driver_info field
>> defined in linux/mod_devicetable.h as unsigned long).
>> Making this 64bit is IMO an extensive API change (if even possible).
>> I guess this is not the way to go.
>>
>> Could USB maintainers please help to advise what is the correct
>> solution? I am not familiar with the USB driver model here
>> and I see no easy way how it can be solved by a trivial static
>> allocation inside the USB storage driver.
>>
>> Someone will need a new quirks flag in the future anyway... :)
>
> I can think of only one way to accomplish this on 32-bit systems: Change
> the driver_info field from a bit array to an index into a static table
> of 64-bit flags values. Each unusual_devs structure would have its own
> entry in this table. As far as I can tell, the other unusual_*.h tables
> could retain their current driver_info interpretations, since no new
> quirk bits are likely to be relevant to them.
>
> Making this change would be an awkward nuisance, but it should be
> doable.
Hm, yes, thanks for the idea,that is a possible solution.
It will need to modify all unusual macros, though. Just I am not sure I want
to spent time patching all the drivers as I have not way how to test it.
Thanks,
Milan
Powered by blists - more mailing lists