From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752067AbdK0Les (ORCPT ); Mon, 27 Nov 2017 06:34:48 -0500 Received: from mga05.intel.com ([192.55.52.43]:38458 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751710AbdK0Leq (ORCPT ); Mon, 27 Nov 2017 06:34:46 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,464,1505804400"; d="asc'?scan'208";a="180626061" From: Felipe Balbi To: John Keeping Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Greg Kroah-Hartman , Vincent Pelletier , Jim Lin Subject: Re: [PATCH] usb: f_fs: Drop check on Reserved1 field on OS_DESC_EXT_COMPAT In-Reply-To: <20171113181954.41d47cb8.john@metanate.com> References: <20171109163429.8697-1-john@metanate.com> <874lq2zaag.fsf@linux.intel.com> <20171110183408.4fc19913.john@metanate.com> <877euue99q.fsf@linux.intel.com> <20171113181954.41d47cb8.john@metanate.com> Date: Mon, 27 Nov 2017 13:34:20 +0200 Message-ID: <87vahwklab.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, John Keeping 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=3D1 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 =3D=3D= 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: stable@vger.kernel.org > Signed-off-by: John Keeping > --- > 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/func= tion/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; >=20=20 > if (len < sizeof(*d) || > - d->bFirstInterfaceNumber >=3D ffs->interfaces_count || > - !d->Reserved1) > + d->bFirstInterfaceNumber >=3D ffs->interfaces_count) > return -EINVAL; > + if (d->Reserved1 !=3D 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 =3D 1; > + } exactly like that. Care to send as a formal patch so I can apply? =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAlob+D0ACgkQzL64meEa mQYaChAA1lj9Y+urbUhCAHAqoAA3zZIRJfd4sm15LkMOmNojJG6Z+mR3cj7vfYGn hOA6/DGtaDDCqNfOqxnWsW1DwrEuSe0dHDWK5sQ6Bn9CEQHnHifkW+4hsYNNa8Dx /E+sivgUbGJSTLCD3Uk7R2OGlPXybz2QeJIoheng2XUakRNTsThiTEemwaR6Wu0i NTs2qPaOajc3hakKDgURtvNgV0yRbd+KOQXWG03vxHNpZDudeOjj/Hcceg4xgs6h BxTMC983QXWOehO0KeZ8s9jlE5WvVuQ5j3/WA4H9EJdNahPNKLeAyA7oYQemppE2 j7lZhdmA+9pOZ5/iuPnSpkBux+QHjJgNEMjhqOQCBMxTBd0mVxJXHixBNVOshgUM nJrC9DO+CJZisgsCkwBismi8W3Quy3EdX0e5JcgELsx0vSHamlqnMjzA+P60sIdw Jzg4gV6c7txSKzVTtLzt5Erk4jynTgQ+AfkmoGa3z8CQ2IaBX7LNkooZoltzofNy Lyjh7YH7XnpXqVA6MA5GkDQZ9/MCsB88TpPx7Jc0CzmCskqVADp7bl0EFhPo5sT+ UozxYc2y8DVZJOLind9IJ/pjMVB2lCj+3j3gtNA3fzZfkS/0Pbb75kyKDCBLXELg qCDNHzjIlkDIw3/GiC+D2IrqZ6+Ao3MCz56TNxjPar+I+6ueohM= =k3lJ -----END PGP SIGNATURE----- --=-=-=--