[U-Boot] [PATCH] [RFC] usb: storage: Limit transfer size to 120 kiB

[Bin Meng]

On Mon, Sep 16, 2019 at 6:24 AM Marek Vasut <marek.vasut at gmail.com> wrote:
>
> Due to constant influx of more and more weird and broken USB sticks,
> do as Linux does in commit 779b457f66e10de3471479373463b27fd308dc85
>
>     usb: storage: scsiglue: further describe our 240 sector limit
>
>     Just so we have some sort of documentation as to why
>     we limit our Mass Storage transfers to 240 sectors,
>     let's update the comment to make clearer that
>     devices were found that would choke with larger
>     transfers.
>
>     While at that, also make sure to clarify that other
>     operating systems have similar, albeit different,
>     limits on mass storage transfers.
>
> And reduce the maximum transfer length of USB storage to 120 kiB.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas at gmail.com>
> Cc: Bin Meng <bmeng.cn at gmail.com>
> Cc: Simon Glass <sjg at chromium.org>
> ---
>  common/usb_storage.c | 27 +++++++++++++++++----------
>  1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/common/usb_storage.c b/common/usb_storage.c
> index 8c889bb1a6..130eab7832 100644
> --- a/common/usb_storage.c
> +++ b/common/usb_storage.c
> @@ -943,21 +943,28 @@ static void usb_stor_set_max_xfer_blk(struct usb_device *udev,
>         int __maybe_unused ret;
>
>  #if !CONFIG_IS_ENABLED(DM_USB)
> -#ifdef CONFIG_USB_EHCI_HCD
>         /*
> -        * The U-Boot EHCI driver can handle any transfer length as long as
> -        * there is enough free heap space left, but the SCSI READ(10) and
> -        * WRITE(10) commands are limited to 65535 blocks.
> +        * Limit the total size of a transfer to 120 KB.
> +        *
> +        * Some devices are known to choke with anything larger. It seems like
> +        * the problem stems from the fact that original IDE controllers had
> +        * only an 8-bit register to hold the number of sectors in one transfer
> +        * and even those couldn't handle a full 256 sectors.
> +        *
> +        * Because we want to make sure we interoperate with as many devices as
> +        * possible, we will maintain a 240 sector transfer size limit for USB
> +        * Mass Storage devices.
> +        *
> +        * Tests show that other operating have similar limits with Microsoft
> +        * Windows 7 limiting transfers to 128 sectors for both USB2 and USB3
> +        * and Apple Mac OS X 10.11 limiting transfers to 256 sectors for USB2
> +        * and 2048 for USB3 devices.
>          */
> -       blk = USHRT_MAX;
> -#else
> -       blk = 20;
> -#endif
> +       blk = 240;
>  #else
>         ret = usb_get_max_xfer_size(udev, (size_t *)&size);

If we blindly set blk to 240 for both non-dm usb and dm usb here,
which makes usb_get_max_xfer_size() useless. Should we completely drop
the usb_get_max_xfer_size() call?

>         if (ret < 0) {
> -               /* unimplemented, let's use default 20 */
> -               blk = 20;
> +               blk = 240;
>         } else {
>                 if (size > USHRT_MAX * 512)
>                         size = USHRT_MAX * 512;
> --

Regards,
Bin