On Fri, Oct 06, 2023 at 02:54:44PM +0200, Milan Broz wrote:
> This patch enables pass-through OPAL command for USB-attached storage
> (which does not support UAS or SCSI security commands).
> All common USB/SATA or USB/NVMe adapters I tested need this patch.
>
> USB mass storage devices that do not support SECURITY IN/OUT SCSI
> commands can support ATA12 pass-thru command.
>
> Internal kernel implementation for OPAL interface currently supports
> only SCSI SECURITY IN/OUT wrapper.
> USB mass storage also turns off SCSI command enumeration, so SECURITY
> IN/OUT in the SCSI layer will be disabled.
>
> Note: sedutils and some other OPAL tools already use ATA-12 pass-thru.
>
> This patch
> - enables SCSI security flag for USB mass storage and UAS device by default.
>
> - adds an optional wrapper to the SCSI layer for the ATA-12 pass-thru
> command as an alternative if SECURITY IN/OUT is unavailable.
>
> In short, the patch runs these steps:
> 1) USB drives (mass-storage, UAS) enables security driver flag by default
> if not disabled by quirk
> 2) SCSI device enumerates SECURITY IN/OUT support. If detected,
> SECURITY ON/OUT wrapper is used (as in the current code).
> If not, new ATA12 pass-thru wrapper is used instead.
> 3) SED OPAL code tries OPAL discovery command for device. If it receives
> correct reply, OPAL is enabled for the device.
>
> With the changes above, the TCG OPAL support works with USB adapters
> that support the ATA-12 command. As kernel OPAL code runs discover
> commands on initialization, on devices that do not support pass-through,
> OPAL remains disabled.
You don't explicitly mention what will happen with devices that don't
support ATA-passthrough. Presumably the ATA12 commands will simply be
rejected and OPAL will not be enabled.
> The patch also adds a quirk flag to disable security commands for particular
> devices if firmware is buggy.
>
> Signed-off-by: Milan Broz <gmazyland@xxxxxxxxx>
> ---
I think it might be better to split this into two patches: One adding
the security driver flag in the USB drivers and one for the SCSI changes
-- those can be added separately through the SCSI tree after the USB
changes have been merged.
I'm not going to try to review the SCSI changes.
> diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
> index f3a53c3eeb45..04211ac803e4 100644
> --- a/drivers/usb/storage/usb.c
> +++ b/drivers/usb/storage/usb.c
> @@ -478,7 +478,7 @@ void usb_stor_adjust_quirks(struct usb_device *udev, u64 *fflags)
> US_FL_INITIAL_READ10 | US_FL_WRITE_CACHE |
> US_FL_NO_ATA_1X | US_FL_NO_REPORT_OPCODES |
> US_FL_MAX_SECTORS_240 | US_FL_NO_REPORT_LUNS |
> - US_FL_ALWAYS_SYNC);
> + US_FL_ALWAYS_SYNC | US_FL_IGNORE_OPAL);
>
> p = quirks;
> while (*p) {
> @@ -567,6 +567,9 @@ void usb_stor_adjust_quirks(struct usb_device *udev, u64 *fflags)
> case 'y':
> f |= US_FL_ALWAYS_SYNC;
> break;
> + case 'z':
> + f |= US_FL_IGNORE_OPAL;
> + break;
> /* Ignore unrecognized flag characters */
> }
> }
You need to add a corresponding entry to the usb-storage.quirks entry in
Documentation/admin-guide/kernel-parameters.txt.
Alan Stern
