Hi,
John Keeping <j...@metanate.com> writes:
> On Mon, 13 Nov 2017 12:57:21 +0200, Felipe Balbi wrote:
>> Good point. Then how about we just force the value to 1 in f_fs.c and
>> remove the check?
>
> That seems reasonable. Something like this?
>
> -- >8 --
> Subject: [PATCH] usb: f_fs: Force Reserved1=1 in OS_DESC_EXT_COMPAT
>
> The specification says that the Reserved1 field in OS_DESC_EXT_COMPAT
> must have the value "1", but when this feature was first implemented we
> rejected any non-zero values.
>
> This was adjusted to accept all non-zero values (while now rejecting
> zero) in commit 53642399aa71 ("usb: gadget: f_fs: Fix wrong check on
> reserved1 of OS_DESC_EXT_COMPAT"), but that breaks any userspace
> programs that worked previously by returning EINVAL when Reserved1 == 0
> which was previously the only value that succeeded!
>
> If we just set the field to "1" ourselves, both old and new userspace
> programs continue to work correctly and, as a bonus, old programs are
> now compliant with the specification without having to fix anything
> themselves.
>
> Fixes: 53642399aa71 ("usb: gadget: f_fs: Fix wrong check on reserved1 of
> OS_DESC_EXT_COMPAT")
> Cc: sta...@vger.kernel.org
> Signed-off-by: John Keeping <j...@metanate.com>
> ---
> drivers/usb/gadget/function/f_fs.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_fs.c
> b/drivers/usb/gadget/function/f_fs.c
> index 652397eda6d6..520a96e7ef9a 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -2282,9 +2282,18 @@ static int __ffs_data_do_os_desc(enum ffs_os_desc_type
> type,
> int i;
>
> if (len < sizeof(*d) ||
> - d->bFirstInterfaceNumber >= ffs->interfaces_count ||
> - !d->Reserved1)
> + d->bFirstInterfaceNumber >= ffs->interfaces_count)
> return -EINVAL;
> + if (d->Reserved1 != 1) {
> + /*
> + * According to the spec, Reserved1 must be set to 1
> + * but older kernels incorrectly rejected non-zero
> + * values. We fix it here to avoid returning EINVAL
> + * in response to values we used to accept.
> + */
> + pr_debug("usb_ext_compat_desc::Reserved1 forced to
> 1\n");
> + d->Reserved1 = 1;
> + }exactly like that. Care to send as a formal patch so I can apply? -- balbi
signature.asc
Description: PGP signature
