[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-usb
Subject:    Re: [RFC PATCH v2] usb: gadget: f_fs: Fix incorrect version checking of OS descs
From:       Yuta Hayama <hayama () lineo ! co ! jp>
Date:       2023-02-28 7:11:46
Message-ID: 9adf6c33-060f-3654-25fa-3cf70a32e918 () lineo ! co ! jp
[Download RAW message or body]

Hi Andrzej,

On 2023/02/28 0:51, Andrzej Pietrasiewicz wrote:
> After all the discussion we've had, I agree that the initial approach in your
> patch to allow 0x100 as a legitimate value in the check performed in
> __ffs_do_os_desc_header() is correct.
> 
> Whatever arrives through ep0 is in little endian format. So 0x0100 will be
> stored as 0x0001, but then u16 bcd_version = le16_to_cpu(desc->bcdVersion);
> will ensure bcd_version equals 0x0100 no matter what (and regardless of
> machine's endinanness) and we *should* be comparing it with 0x0100.

Thank you for your understanding. I'm sorry if my explanation was not clear.

> I would probably change the message generated by this portion:
> 
> +        pr_warn("bcdVersion is expected to be 0x100, but it was 0x%x. "
> +            "Pass for compatibility, but fix your user space driver.\n",
> +            bcd_version);
> 
> to, say:
> 
> pr_warn("bcdVersion must be 0x0100, stored in Little Endian order. "
>     "Userspace driver should be fixed, accepting 0x0001 for compatibility.\n");

It helped me because I am not good at English. I would like to replace the
message with this in the v3 patch.

> and there's no need to format-print bcd_version given it is 0x1
> in this branch of the "if" statement.

Yes, indeed. Aside from the wording, the format specifier was not needed.


Regards,

Yuta Hayama
[prev in list] [next in list] [prev in thread] [next in thread]